Add --skip-existing support for Artifactory PyPI repository type

[goodtune]

We use Artifactory for a variety of purposes, and one of the supported repository types is PyPI.

We typically have permissions set such that publishers are prevented from overwriting packages. The response from Artifactory is different to that which PyPI and pypiserver deliver so it is not possible to use the --skip-existing switch with twine when Artifactory is your server.

This pull request refactors the skip_upload (internal API) such that:

• it is only executed when the user has provided the --skip-existing switch at the command line
• internally it checks a series of gates that will make it easier to special case other implementations
• has been renamed ignore_upload_failure because it was never avoiding an upload, only dealing with the result of an attempt to upload

This is backwards compatible and makes no changes to the public API.

[codecov]

Codecov Report

Merging #326 into master will decrease coverage by 0.24%.
The diff coverage is 61.53%.

@@            Coverage Diff             @@
##           master     #326      +/-   ##
==========================================
- Coverage   68.84%   68.59%   -0.25%     
==========================================
  Files          12       12              
  Lines         597      605       +8     
  Branches       95      100       +5     
==========================================
+ Hits          411      415       +4     
  Misses        158      158              
- Partials       28       32       +4

Impacted Files	Coverage Δ
twine/commands/upload.py	64.51% <61.53%> (-1.37%)	⬇️
twine/wininst.py	29.72% <0%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f5ca338...e94648e. Read the comment docs.

[brainwane]

Hi, @goodtune and thank you for this PR! I appreciate the the clear explanation in the comment and here in the PR in particular.

I'm taking a look at this now.

Clearly the various Python package indexes could be a bit more congruent with each other when it comes to an upload API. I've started pypa/packaging-problems#128 for proposing a PEP to get some basics in place. Do you have a JFrog Artifactory contact I could reach out to?

[goodtune]

I don't actually have a name of anyone at JFrog – I developed against our on-premise instance by capturing the response under the (albeit naively construed) scenarios that correspond to the behaviours already being tested.

Thanks for creating the ticket to attack the broader problem of specification-by-implementation.

[goodtune]

Hi @brainwane is there anything else I can do? Are there any implementation concerns?

[brainwane]

Hi @goodtune and sorry for the delay. Yesterday I was putting out Twine 1.11.0 and I just started looking at your PR, and haven't delved into it as deeply as I would like yet. I would like to confer with fellow maintainers before accepting this -- @sigmavirus24 especially.

I can tell you a few things you could do to make this a little easier to review (I'm sorry to ask but I do think they will help):

• double-check whether, where you've changed this skips-existing test, it would be good to add either an additional test or additional comments to clarify which package indexes' behavior is being tested in each test
• provide us with an example (perhaps we could use it as a fixture) of the JSON response Artifactory gives in this situation
• fix the newline you've removed at the end of AUTHORS (but thank you for adding yourself!)

Thank you. I am sorry for the wait.

[sigmavirus24]

A quick scan of this makes me confident this is a good change going forward. I think all of @brainwane's suggestions are good ones.

We could use betamax to record an interaction with Artifactory, but that seems like a lot of set-up for @goodtune to do.

It would be good for codecov to be happy with this patch, although, I think @brainwane has had other issues with it.

[goodtune]

I typically use requests and responses however because the test suite already used pretend I didn't want to replace that at the same time.

I'd consider dedicating some time to that in a future PR.

[niothiel]

FWIW: I support this enhancement. :)

[theacodes]

Closing in favor of #363