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

#3310 makes twine --skip-existing option useless #3482

Closed
mayeut opened this issue Mar 30, 2018 · 4 comments
Closed

#3310 makes twine --skip-existing option useless #3482

mayeut opened this issue Mar 30, 2018 · 4 comments
Labels

Comments

@mayeut
Copy link
Contributor

mayeut commented Mar 30, 2018

twine checks for message starting with File already exists:
https://github.com/pypa/twine/blob/bcb73e025102565c395e0bf322faebd7f72e76f7/twine/commands/upload.py#L58-L61
Message has been changed to one starting with The filename or contents already exist in #3310:
https://github.com/pypa/warehouse/blob/969695d030aef8b11d5b052e8731a80746c404c8/warehouse/forklift/legacy.py#L1083-L1094

c.f. pypa/twine#332
c.f. failing deployment: https://travis-ci.org/mayeut/gcovr/builds/360424325

@brainwane
Copy link
Contributor

@sigmavirus24 @jonparrott @di I'm offline much of today, could one of you take a look at this?

Thanks for the report, @mayeut.

@mayeut
Copy link
Contributor Author

mayeut commented Mar 31, 2018

@brainwane, given you're comment in pypa/twine#332 (comment) saying it shall probably be fixed in twine, I guess one thing that could be done is add a comment in warehouse. Maybe something like:
# changing the message might break twine --skip-existing
This will prevent further issues from popping up. At least, there can be some coordination before an issue arises (e.g. release twine with new message support then change the message).

@sigmavirus24
Copy link
Contributor

With my twine hat on, I've always been of the opinion that our current checking around this is fragile and sucks. If Warehouse can provide us with something better that would be fantastic. It seems like Warehouse, however, has traded one plain-text string for another. For this API, I understand returning plain-text/html, but I think if the client says they accept JSON, a JSON payload would be leagues better. Just my 2cents

@di
Copy link
Member

di commented Apr 2, 2018

Hmm, this is unfortunate. Seems like we should revert the error message change so that this behavior is restored for older Twine clients, and then work on improving the brittleness of this in Twine.

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

No branches or pull requests

4 participants