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

Twine check fails to warn on invalid classifier #976

Open
pombredanne opened this issue Jan 11, 2023 · 8 comments · May be fixed by #1166
Open

Twine check fails to warn on invalid classifier #976

pombredanne opened this issue Jan 11, 2023 · 8 comments · May be fixed by #1166

Comments

@pombredanne
Copy link

Your Environment

Using twine:

$ twine --version
twine version 4.0.2 (importlib-metadata: 6.0.0, keyring: 23.13.1, pkginfo: 1.9.6, requests:
2.28.1, requests-toolbelt: 0.10.1, urllib3: 1.26.13)

I have a setup.cfg with this classifier:
Development Status :: 4 - Beta
there is an extra space before the 4

If I build a wheel and run twine check, it passes (see attached wheel where the .zip extension has been added to make this tracker happy)
tracecode_toolkit_strace-0.21.0-py3-none-any.whl.zip

$ twine check dist/tracecode_toolkit_strace-0.21.0-py3-none-any.whl --strict
Checking dist/tracecode_toolkit_strace-0.21.0-py3-none-any.whl: PASSED

If I try to upload this to PyPI I get this error:

$ twine upload dist/tracecode_toolkit_strace-0.21.0-py3-none-any.whl 
Uploading distributions to https://upload.pypi.org/legacy/
Uploading tracecode_toolkit_strace-0.21.0-py3-none-any.whl
100% ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 97.6/97.6 kB • 00:00 • 458.3 kB/s
WARNING  Error during upload. Retry with the --verbose option for more details.                     
ERROR    HTTPError: 400 Bad Request from https://upload.pypi.org/legacy/                            
         Invalid value for classifiers. Error: Classifier 'Development Status ::  4 - Beta' is not a
         valid classifier.                           

I would expect the check to fail if the upload would fail.

@sigmavirus24
Copy link
Member

Spiritually I think this is a duplicate of #430. Regardless it's part of the overall theme of "twine doesn't check things that we have to reimplement because PyPI doesn't have a shared library with these validations"

@bhrutledge bhrutledge closed this as not planned Won't fix, can't repro, duplicate, stale Jan 11, 2023
@pombredanne
Copy link
Author

Spiritually I think this is a duplicate of #430. Regardless it's part of the overall theme of "twine doesn't check things that we have to reimplement because PyPI doesn't have a shared library with these validations"

@sigmavirus24 You may have missed that PyPI now has a shared library for classifiers with https://github.com/pypa/trove-classifiers which is also used in warehouse since @di merged it in pypi/warehouse#7582 ?

@bhrutledge
Copy link
Contributor

@pombredanne I wasn't aware of that library, which I think does re-open the door for implementing this in Twine. That said, there's been a lot of discussion about how to add check behavior (e.g. #848), and there's a whole project for check improvements.

That said: given that PyPI rejects the upload, I guess I'm not sure how much value there is in adding it to twine check. Perhaps naively, it seems like the only savings is the upload time, and the remediation would be the same. By contrast, IIRC the README validation in twine check is behavior that doesn't exist in Warehouse.

@pombredanne
Copy link
Author

@bhrutledge Thanks for reopening!

You wrote:

That said: given that PyPI rejects the upload, I guess I'm not sure how much value there is in adding it to twine check. Perhaps naively, it seems like the only savings is the upload time, and the remediation would be the same.

FWIW, the problem for me is that when PyPI rejects an upload, it is too late: the code has already been validated, tests have run, the repo is tagged, a release has been created and wheels/sdist built, typically all automatically from a tag.

A failure to upload means I need to update classifiers and respin a mostly ugly dot release for the sole purpose of fixing the classifiers. And I eventually want to yank the previous release or tag so there is no dangling tag that's not published to PyPI.

I would much prefer a twine check failing during a test run in the CI.

@sigmavirus24
Copy link
Member

Many people will create a pre-release tag that will upload to Test PyPI. Then you can make changes before making a final release. This too can be automated the same way your normal release flow is. You can even see if it uploads to Test PyPI and then optionally release to PyPI as well for people to install and test from if you want.

This doesn't need to be solved by Twine.

There is far too much warehouse checks that we cannot and every inch we move towards doing that, is more maintenance burden and momentum we have to resist reimplementing (poorly) those checks. (Especially since Warehouse doesn't externalize those often or ever)

@bhrutledge
Copy link
Contributor

@pombredanne Thanks for the details of your scenario. That makes sense, and I see how an upload failure is cumbersome.

There is far too much warehouse checks that we cannot and every inch we move towards doing that, is more maintenance burden and momentum we have to resist reimplementing (poorly) those checks. (Especially since Warehouse doesn't externalize those often or ever)

This occurred to me as well. Even if we enhance check (which I'm open to), I don't think it will ever be comprehensive, unless Warehouse completely externalizes its validation. Thus, there's always a non-trivial chance that an upload will fail, and require the remediation that you described.

Many people will create a pre-release tag that will upload to Test PyPI. Then you can make changes before making a final release. This too can be automated the same way your normal release flow is. You can even see if it uploads to Test PyPI and then optionally release to PyPI as well for people to install and test from if you want.

I think this is the most robust solution, though it would require re-working your release automation. I think you could even upload a pre-release to PyPI.

@di
Copy link
Member

di commented Jan 23, 2023

By contrast, IIRC the README validation in twine check is behavior that doesn't exist in Warehouse.

Unless it's changed significantly since it was implemented, the README validation is the same as PyPI does on upload.

I would much prefer a twine check failing during a test run in the CI.

I agree, and this is precisely the use case for twine check: to ensure a given project could be uploaded to PyPI successfully at any given time.

There is far too much warehouse checks that we cannot and every inch we move towards doing that, is more maintenance burden and momentum we have to resist reimplementing (poorly) those checks. (Especially since Warehouse doesn't externalize those often or ever)

The current plan is to externalize them into a reusable library, it just hasn't happened yet: pypa/packaging#147

browniebroke added a commit to browniebroke/twine that referenced this issue Oct 12, 2024
@browniebroke browniebroke linked a pull request Oct 12, 2024 that will close this issue
3 tasks
@browniebroke
Copy link

I've just been bitten by this issue. As the conversation seems to indicate that it would be a welcome addition, I went ahead with: #1166

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

Successfully merging a pull request may close this issue.

5 participants