leaving out a trailing slash in the pypi URL causes twine to fail with a 410

[brainwane]

Given a [test] repository described as https://test.pypi.org/legacy in a .pypirc file, twine upload --repository test dist/* fails with HTTPError: 410 Client Error: This API has moved to https://test.pypi.org/legacy/. See TODO for more information. for url: https://test.pypi.org/pypi.

<runciter> though i'd expect a 301 or 302
<runciter> not a very big deal because the correct URL is in the error message
<runciter> (maybe not a deal at all)
...
<sumanah> coming back to the "missing trailing slash" thing. To make sure I have this right: you had a [test] section in your .pypirc that had the right URL but missing a trailing slash, correct?
<runciter> sumanah: yep!

Repro case with .pypirc: https://gist.github.com/markrwilliams/01a235daeee2d6a0068a22d2faa7b2cb

Could we in this case fix the trailing slash to make life easier for users?

[sigmavirus24]

So the unfun bit is that we get a 410 but 410 is pretty generic. If Warehouse returned a JSON body, we could perhaps introspect that and retry with a trailing slash. We could also parse the URL, detect test.pypi.org and pypi.org and check that the original (not parsed) URL ends with a /. If it doesn't, we can warn the user, append it and move on.

[sigmavirus24]

And the 410 is from Warehouse, so making it a 301 or a 302 is a feature request for Warehouse. 😄

[dstufft]

I don't think redirects work reliably because the API is a POST request. At the very least I'm really sure that distutils/setuptools won't follow the redirect even if twine will.

[sigmavirus24]

Twine doesn't automagically follow redirects on POSTs for many reasons, not the least of which is that we're using file-like objects whose positions don't auto-reset. We could follow them manually easily if we wanted to.

I think, however, that the better user experience is to special case this with PyPI and handle it in the Repository object before we do any HTTP requests so that twine is as speedy as it can be.

[patrick-motard]

Had I not found this issue I would not have been able to upload.

No trailing '/' in .pypirc:

[pypitest]
repository=https://test.pypi.org/legacy

Resulting error:

twine upload -r pypitest dist/*
Uploading distributions to https://test.pypi.org/legacy
Uploading polybar_reload-0.1.0-py3-none-any.whl
100%|███████████████████████████████████████████████| 4.65k/4.65k [00:00<00:00, 18.8kB/s]
RedirectDetected: "https://test.pypi.org/legacy" attempted to redirect to "https://test.pypi.org/legacy/" during upload. Aborting...

No idea what that means... Did a bit of googling.. got lucky that this thread showed up as a match for "RedirectDetected". Notice a mention of trailing '/'.. No other search results come close to relating to this issue.

Added trailing '/' in .pypirc:

[pypitest]
repository=https://test.pypi.org/legacy/

twine upload -r pypitest dist/*
Uploading distributions to https://test.pypi.org/legacy/
Uploading polybar_reload-0.1.0-py3-none-any.whl
100%|███████████████████████████████████████████████| 4.65k/4.65k [00:01<00:00, 3.21kB/s]
Uploading polybar-reload-0.1.0.tar.gz
100%|███████████████████████████████████████████████| 4.02k/4.02k [00:01<00:00, 3.89kB/s]

Works. I would really like to see the error handling improved.

Side question:

If I'm using pip and virtualenv. Package deployment seems to need a setup.py though. Why can't i use my Pipfile instead?

[sigmavirus24]

@patrick-motard thanks for the feedback. As for your side question, that would be better brought up on https://github.com/pypa/packaging-problems (if it hasn't been discussed there already).

[jmillxyz]

If anyone else runs into the same issue, I also neglected a trailing slash and got a 404 on our devpi instance (anonymized to protect the innocent):

...
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed

100 38096    0  1381  100 36715   6806   176k --:--:-- --:--:-- --:--:--  177k
Uploading distributions to https://devpi.our-url.com/path
Uploading software-0.10.9-py3-none-any.whl

100% 45.7k/45.7k [00:00<00:00, 169kB/s] 
HTTPError: 404 Client Error: Not Found for url: https://devpi.our-url.com/path
Exited with code 1

Adding the trailing slash fixed things. Thanks for making this issue!

[brainwane]

Thanks, @bhrutledge!

[brainwane]

So the unfun bit is that we get a 410 but 410 is pretty generic. If Warehouse returned a JSON body, we could perhaps introspect that and retry with a trailing slash. We could also parse the URL, detect test.pypi.org and pypi.org and check that the original (not parsed) URL ends with a /. If it doesn't, we can warn the user, append it and move on.

I think, however, that the better user experience is to special case this with PyPI and handle it in the Repository object before we do any HTTP requests so that twine is as speedy as it can be.

We got a request for this at a recent sprint night from a new contributor. @bhrutledge what do you think?

[bhrutledge]

@brainwane Seems reasonable, and per my comment in #460:

I would probably do that in utils.normalize_repository_url by appending a trailing slash if the URL is in _HOSTNAMES.

That said, I'm slightly wary of implicitly altering the configured repositories, and continuing down the path of special-casing. In light of #499 & #509, I'm wondering if a "better" approach would be to sanity check the repository URL prior to upload, and abort with a helpful error message.

Something like:

if "pypi.org" in url and url not in [DEFAULT_REPOSITORY, TEST_REPOSITORY]:
    print(
        f"The configured repository {url} is not a known PyPI repository.\n"
        f"Did you mean {DEFAULT_REPOSITORY} or {TEST_REPOSITORY}?\n"
        f"See {PYPIRC_DOCS} for details."
    )
    return

This would put up a temporary roadblock, but would encourage folks to have a "correct" configuration. If that sounds like a good idea, I'm inclined to open that as a new enhancement issue, and close this one.

Maybe the @pypa/twine-maintainers have opinions?

[bhrutledge]

Settings.check_repository_url might be a good place for more robust pre-upload checks.

[jaraco]

SGTM

[ussserrr]

Adding trailing / helps

[dukecat0]

@bhrutledge Do you think this will work?

We could provide this message:

f"The configured repository {url} is not a known PyPI repository.\n"
f"Did you mean {DEFAULT_REPOSITORY} or {TEST_REPOSITORY}?\n"

Then, we could ask users whether they want to update the configured repository in the twine directly:

If you are uploading distribution(s) to PyPI or TestPyPI,
do you want twine to add trailing `/` to the configured repository {url}
so twine can upload it properly? [y/n]: 

If users input yes, we will add trailing / to the configured repository.

Therefore, users will know that twine has modified their .pypirc.

[sigmavirus24]

Then, we could ask users whether they want to update the configured repository in the twine directly:

I would think this is helpful but I would also give people the ability to specify the config file to save it to. Some people might want to diff the two files to ensure it's only changed the URL. I would also remove If you are uploading distribution(s) to PyPI or TestPyPI, because ostensibly they've already answered "Yes" (or something along those lines) so all we need to say is We're appending a / to the URL ({url}) in your {configfilepath}. Do you want us to save this configuration for you for future use? [y/N]: and then say Where should we save this? [{configfilepath}]: I'm explicitly saying {configfilepath} because a user could be using --config and we should update that and not ~/.pypirc.

[bhrutledge]

@meowmeowmeowcat @sigmavirus24 I appreciate the suggestions to help the user, but this also feels like a complex bit of behavior to implement, test, and review. For example, thus far, Twine does not write to the configuration file. It also seems like this issue doesn't come up very often.

With all that in mind, I would rather start with the lighter-weight "help" message proposed in #310 (comment), maybe extending it to mention the configuration file, or suggesting --verbose. Hopefully, that will encourage folks to fix their configuration.

[sigmavirus24]

@meowmeowmeowcat @sigmavirus24 I appreciate the suggestions to help the user, but this also feels like a complex bit of behavior to implement, test, and review. For example, thus far, Twine does not write to the configuration file. It also seems like this issue doesn't come up very often.

Generally the consensus around developer tooling these days is "If you know how to fix this and can fix it for me, why don't you?" This should be very little effort and complexity.

With all that in mind, I would rather start with the lighter-weight "help" message proposed in #310 (comment), maybe extending it to mention the configuration file, or suggesting --verbose. Hopefully, that will encourage folks to fix their configuration.

I would suggest expanding the very basic message to print a "diff" to help make it clearer what the difference is. Not everyone might understand the phrasing we pick in english, but most should be able to look at a diff and understand what the change necessary is.

[dukecat0]

Probably this could be closed as solved? We have provided an improved redirect message with #812.

[bhrutledge]

@meowmeowmeowcat I haven't closed it because there was some follow up that I wanted to do:

#310 (comment) shows that devpi returns "Not Found" instead of redirecting. I'm inclined to follow up with a commit or PR to handle that case in utils.check_status_code

[bhrutledge]

After taking a closer look, I'm going to close this with no further changes. Unlike the redirect case, a 404 on a URL without a trailing slash doesn't necessarily imply that adding a trailing slash is the proper resolution; for example, it could mean a typo in the URL.