Skip to content
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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: skip upload for sonatype nexus collision #1221

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jafournier
Copy link

@jafournier jafournier commented Jan 24, 2025

Context:
We want to use the --skip-existing option with Sonatype Nexus' pypi.
However when package already exists, Sonatype Nexus is delivering a 400 reponse with a content not handled yet in the code.

The purpose of this PR is to handle such contents.

@woodruffw
Copy link
Member

Thanks for the PR @jafournier.

Two quick questions:

  • Is this different from Sonatype's "Nexus Repository OSS" service? We already support that a few lines above; if this is the new version of that existing service then the old one should probably be removed and replaced with this.
  • Is this error message documented anywhere? I couldn't find it with a quick search on the linked docs. This isn't a hard blocker, but it'd be nice to know if this is actually documented vs. something the Sonatype developers might choose to change.

tests/test_upload.py Outdated Show resolved Hide resolved
@jafournier jafournier force-pushed the feat/skip_upload_for_sonatype_nexus_collisions branch from f0eb0f3 to 05c096b Compare January 24, 2025 16:12
@jafournier jafournier force-pushed the feat/skip_upload_for_sonatype_nexus_collisions branch from 05c096b to aa27805 Compare January 24, 2025 16:15
@jafournier
Copy link
Author

Thanks for the PR @jafournier.

Two quick questions:

* Is this different from Sonatype's "Nexus Repository OSS" service? We already support that a few lines above; if this is the new version of that existing service then the old one should probably be removed and replaced with this.

* Is this error message documented anywhere? I couldn't find it with a quick search on the linked docs. This isn't a hard blocker, but it'd be nice to know if this is actually documented vs. something the Sonatype developers might choose to change.

Thanks for your answer @woodruffw! Indeed the two are the same, consequently I merged the two lines.
We had a change in behavior when our nexus got bumped to 3.70.3.
I think old-nexus users would like twine to keep the compatibility with their old-nexus.
Unfortunately I haven't found any documentation of such content either.

@woodruffw
Copy link
Member

Got it, thanks for checking! This LGTM overall; could you please create a changelog entry for your changes as well? I think this could go under either feature or bugfix (no strong opinion about which).

Instructions for the changelog are here: https://twine.readthedocs.io/en/stable/contributing.html#changelog-entries

@jafournier
Copy link
Author

sure, done.

@sigmavirus24
Copy link
Member

Yeah so someone was going to fix this in Nexus itself. There is at least one closed issue about this.

Copy link
Member

@sigmavirus24 sigmavirus24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nexus was supposed to fix this breakage and there's a closed twine issue about that exact topic

@jafournier
Copy link
Author

jafournier commented Jan 27, 2025

Hello @sigmavirus24, are you refering to #1044 ?
If so this issue is still open, and hasn't been resolved since it's opening, one year ago.

Though I theoretically agree that nexus should homogenize it's error messages and stop changing them, what are the chances the issue would be resolved this coming year in practice?
I you consider the chances are dim, couldn't you accept this tiny modification to enable twine users to use --skip-existing smoothly ?

@sigmavirus24
Copy link
Member

@jafournier fixing things here is the path to increasing curb and complexity. Frankly the right fix here is limiting the skip existing feature to indices that actually fully implement the API(s) we expect as we did with trusted publishing and removing string checking hacks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants