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

Traceback exposed if upload fails #558

Open
palao opened this issue Jun 9, 2022 · 5 comments
Open

Traceback exposed if upload fails #558

palao opened this issue Jun 9, 2022 · 5 comments

Comments

@palao
Copy link

palao commented Jun 9, 2022

Hello,
I got a traceback in flit while trying to publish a package:

$ flit publish
Found 360 files tracked in git                                                                                                                                                              I-flit.sdist
Built sdist: dist/MyPackage-0.5.0.tar.gz                                                                                                                                   I-flit_core.sdist
Copying package file(s) from /tmp/tmpdw358fz9/MyPackage-0.5.0/mypackage                                                                                       I-flit_core.wheel
Writing metadata files                                                                                                                                                                 I-flit_core.wheel
Writing the record of files                                                                                                                                                            I-flit_core.wheel
Built wheel: dist/mypackage-0.5.0-py3-none-any.whl                                                                                                                         I-flit_core.wheel
Using repository at https://upload.pypi.org/legacy/                                                                                                                                        I-flit.upload
Uploading dist/mypackage-0.5.0-py3-none-any.whl...                                                                                                                             I-flit.upload
Traceback (most recent call last):
  File "/home/palao/.virtualenvs/flit-py39/bin/flit", line 8, in <module>
    sys.exit(main())
  File "/home/palao/.virtualenvs/flit-py39/lib/python3.9/site-packages/flit/__init__.py", line 185, in main
    main(args.ini_file, repository, args.pypirc, formats=set(args.format or []),
  File "/home/palao/.virtualenvs/flit-py39/lib/python3.9/site-packages/flit/upload.py", line 268, in main
    do_upload(built.wheel.file, built.wheel.builder.metadata, pypirc_path, repo_name)
  File "/home/palao/.virtualenvs/flit-py39/lib/python3.9/site-packages/flit/upload.py", line 246, in do_upload
    upload_file(file, metadata, repo)
  File "/home/palao/.virtualenvs/flit-py39/lib/python3.9/site-packages/flit/upload.py", line 239, in upload_file
    resp.raise_for_status()
  File "/home/palao/.virtualenvs/flit-py39/lib/python3.9/site-packages/requests/models.py", line 960, in raise_for_status
    raise HTTPError(http_error_msg, response=self)
requests.exceptions.HTTPError: 400 Client Error: Invalid value for requires_dist. Error: Can't have direct dependency: 'somedep @ git+https://github.com/whoever/somedep.git' for url: https://upload.pypi.org/legacy/

the problem is that PyPI rejects it (seems to be related to their policy, see #555).
However my question is: is the display of the traceback a feature, or a bug? Should I make a PR?

@takluyver
Copy link
Member

In that case, the traceback clearly isn't helpful - there's a problem with the package we're uploading, not the uploader code. I'm wary of hiding tracebacks in general, though, because they can be useful to understand what's going on.

Some possible ways to address that:

  1. Extend the checks we do when building to cover such cases - the idea is that we catch things like this before you try to upload to PyPI. Checking requirements is done here. This one is tricky, however, because a requirement specified by direct URL may be OK for a package you're using privately, it just can't go on PyPI.
  2. Hide the traceback specifically when we get status code 400, which hopefully means PyPI is telling us about something wrong with the package. 🤞
  3. Hide the traceback normally but have some way to turn it on (this is inspired by Rust). The downside is that it's a pain for rare errors where it might be fine when you run it again.

@takluyver
Copy link
Member

P.S. Sorry this one went unanswered for so long

@palao
Copy link
Author

palao commented Nov 28, 2022

Thank you, @takluyver for answering and for sharing this clear analysis. I really appreciate your comments: they are very insightful.
No worries: it is not a critical thing... still I think it should be fixed, and I agree, the big problem I see here is that the traceback can be confusing. The error is correctly describing what happens, but I needed a while to understand that flit was doing the right thing and that it was my ignorance about the PyPI policy what caused the problem.

I would be in favour of not doing a very clever checking. I don't see a practical way to address your point 1.
Your reasoning about hiding tracebacks is convincing (your point 3): it can be a painful default. Still it is more appealing than point 1. I use this strategy sometimes.

My winner is point 2: if flit is doing things right and the hosting is legitimately answering "No, you can't do that", I think this is a case that should be properly addressed by flit. Simply preventing the exception to propagate and printing an error similar to what's displayed in the last line should be enough (maybe adding some more generic explanation).

What do you think? Should I do a PR?

@takluyver
Copy link
Member

I think that's reasonable, yeah.

What we do for other scenarios like this is that the main CLI code catches a group of errors where we just want to display the message, not the traceback - e.g. here it is for flit build:

flit/flit/__init__.py

Lines 178 to 182 in f5704ea

try:
main(args.ini_file, formats=set(args.format or []),
gen_setup_py=gen_setup_py())
except(common.NoDocstringError, common.VCSError, common.NoVersionError) as e:
sys.exit(e.args[0])

So I would define a new exception like UploadRejectedError, transform HTTP 400 errors into this, and then apply a similar mechanism to flit publish.

@palao
Copy link
Author

palao commented Nov 28, 2022

I like it. It makes sense.

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

No branches or pull requests

2 participants