-
Notifications
You must be signed in to change notification settings - Fork 88
fix: stabilize RSS feed GUIDs for multi-arch wheel publication #1382
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
ryanpetrello
wants to merge
2
commits into
pulp:main
Choose a base branch
from
ryanpetrello:fix/rss-guid-stability
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| RSS feed GUIDs are now stable when additional platform wheels arrive for the same release. GUIDs only change when a new PEP 427 build tag is introduced, preventing feed readers from showing duplicate entries for multi-architecture builds. | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| import pytest | ||
|
|
||
| from pulp_python.app.pypi.feeds import _build_tag_fragment | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "filenames, expected", | ||
| [ | ||
| ([], ""), | ||
| (["shelf-reader-0.1.tar.gz"], ""), | ||
| (["shelf_reader-0.1-py2-none-any.whl"], ""), | ||
| ( | ||
| ["docling_parse-7.19.1-1-cp312-cp312-linux_x86_64.whl"], | ||
| "#builds=1", | ||
| ), | ||
| ( | ||
| [ | ||
| "docling_parse-7.19.1-1-cp312-cp312-linux_x86_64.whl", | ||
| "docling_parse-7.19.1-1-cp312-cp312-linux_aarch64.whl", | ||
| "docling_parse-7.19.1-1-cp312-cp312-linux_ppc64le.whl", | ||
| ], | ||
| "#builds=1", | ||
| ), | ||
| ( | ||
| [ | ||
| "ctranslate2-4.5.0-1-cp312-cp312-linux_x86_64.whl", | ||
| "ctranslate2-4.5.0-2-cp312-cp312-linux_x86_64.whl", | ||
| ], | ||
| "#builds=1,2", | ||
| ), | ||
| ( | ||
| [ | ||
| "foo-1.0-1-cp312-cp312-linux_x86_64.whl", | ||
| "foo-1.0.tar.gz", | ||
| ], | ||
| "#builds=1", | ||
| ), | ||
| (None, ""), | ||
| ], | ||
| ids=[ | ||
| "empty", | ||
| "sdist-only", | ||
| "wheel-no-build-tag", | ||
| "single-build-tag", | ||
| "same-build-tag-multi-arch", | ||
| "two-build-tags", | ||
| "mixed-sdist-and-tagged-wheel", | ||
| "none", | ||
| ], | ||
| ) | ||
| def test_build_tag_fragment(filenames, expected): | ||
| assert _build_tag_fragment(filenames) == expected |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,3 @@ | ||
| mock | ||
| pytest<8 | ||
| pytest-django |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this feature is one you requested, but I wonder if we are deviating from PyPI's behavior. Can you show me an example of a package on PyPI that updates their entries based on build-tags?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a pretty good point 馃 - we may be coloring this RSS implementation with our desired behavior, and not with parity in terms of what PyPI actually does.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, @mprpic, we may actually be at an impasse here. Looking at what PyPI actually exposes in its RSS feed, it doesn't provide a
<guid>element, which means it just falls back to the<link>.PyPI's link is https://pypi.org/project/{name}/{version}/ -- one entry per (name, version), no timestamp, no build tag.
So if we're modeling this after the way that PyPI behaves, the link is the GUID; adding more wheels to an existing release doesn't create a new RSS entry.
So even though the original limitation you discovered doesn't work the way we expected it to, it matches what PyPI does (and I understand why Pulp maintainers would want to mirror the PyPI implementation).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at an example it seems that PyPI doesn't even use GUID: https://pypi.org/rss/project/tensorflow/releases.xml
On the other hand maybe this is a limitation of PyPI and their RSS feeds would be more useful with this.