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

AUTHORS

@@ -19,3 +19,4 @@ Anna Martelli Ravenscroft <annaraven@gmail.com>
 Sumana Harihareswara <sh@changeset.nyc>
 Dustin Ingram <di@di.codes> (https://di.codes)
 Jesse Jarzynka <jesse@jessejoe.com> (http://jessejoe.com)
+Gary Reynolds <gary.reynolds@optiver.com.au>

tests/test_upload.py

@@ -13,6 +13,7 @@
 # limitations under the License.
 from __future__ import unicode_literals
 
+import json
 import os
 import textwrap
 
@@ -131,40 +132,46 @@ def test_deprecated_repo(tmpdir):
 
 def test_skip_existing_skips_files_already_on_PyPI(monkeypatch):
     response = pretend.stub(
+        headers={},
         status_code=400,
         reason='A file named "twine-1.5.0-py2.py3-none-any.whl" already '
                'exists for twine-1.5.0.')
 
     pkg = package.PackageFile.from_filename(WHEEL_FIXTURE, None)
-    assert upload.skip_upload(response=response,
-                              skip_existing=True,
-                              package=pkg) is True
+    assert upload.ignore_upload_failure(response=response, package=pkg) is True
 
 
 def test_skip_existing_skips_files_already_on_pypiserver(monkeypatch):
     # pypiserver (https://pypi.org/project/pypiserver) responds with a
     # 409 when the file already exists.
     response = pretend.stub(
+        headers={},
         status_code=409,
         reason='A file named "twine-1.5.0-py2.py3-none-any.whl" already '
                'exists for twine-1.5.0.')
 
     pkg = package.PackageFile.from_filename(WHEEL_FIXTURE, None)
-    assert upload.skip_upload(response=response,
-                              skip_existing=True,
-                              package=pkg) is True
+    assert upload.ignore_upload_failure(response=response, package=pkg) is True
 
 
-def test_skip_upload_respects_skip_existing(monkeypatch):
+def test_skip_existing_skips_files_already_on_artifactory(monkeypatch):
     response = pretend.stub(
-        status_code=400,
-        reason='A file named "twine-1.5.0-py2.py3-none-any.whl" already '
-               'exists for twine-1.5.0.')
+        headers={'X-Artifactory-Id': '1234567890abcdef'},
+        status_code=403,
+        reason='Forbidden',
+        content=json.dumps({
+            "errors": [
+                {
+                    "status": 403,
+                    "message": "Not enough permissions to overwrite artifact "
+                               "'twine-1.5.0-py2.py3-none-any.whl' (user "
+                               "'user' needs DELETE permission)."
+                },
+            ],
+        }))
 
     pkg = package.PackageFile.from_filename(WHEEL_FIXTURE, None)
-    assert upload.skip_upload(response=response,
-                              skip_existing=False,
-                              package=pkg) is False
+    assert upload.ignore_upload_failure(response=response, package=pkg) is True
 
 
 def test_values_from_env(monkeypatch):

twine/commands/upload.py

@@ -53,22 +53,43 @@ def find_dists(dists):
     return group_wheel_files_first(uploads)
 
 
-def skip_upload(response, skip_existing, package):
-    filename = package.basefilename
-    # NOTE(sigmavirus24): Old PyPI returns the first message while Warehouse
-    # returns the latter. This papers over the differences.
-    msg = ('A file named "{0}" already exists for'.format(filename),
-           'File already exists')
+def ignore_upload_failure(response, package):
     # NOTE(sigmavirus24): PyPI presently returns a 400 status code with the
     # error message in the reason attribute. Other implementations return a
-    # 409 status code. We only want to skip an upload if:
-    # 1. The user has told us to skip existing packages (skip_existing is
-    #    True) AND
-    # 2. a) The response status code is 409 OR
+    # 409 status code.
+    # NOTE(goodtune): Artifactory v5.9 returns a 403 status code with a JSON
+    # payload when the file exists and the user does not have permissions to
+    # replace the package.
+    # We only want to skip an upload if:
+    # 1. The user has told us to skip existing packages (checked prior to
+    #    calling this function) AND
+    # 2. a) The response status code is 409; OR
     # 2. b) The response status code is 400 AND it has a reason that matches
-    #       what we expect PyPI to return to us.
-    return (skip_existing and (response.status_code == 409 or
-            (response.status_code == 400 and response.reason.startswith(msg))))
+    #       what we expect PyPI to return to us; OR
+    # 2. c) The response status code is 403 AND it has a reason that matches
+    #       what we expect Artifactory to return to us.
+
+    # Artifactory provides a custom header we can look for to ensure this
+    # logic is only employed against their implementation.
+    if 'X-Artifactory-Id' in response.headers and response.status_code == 403:
+        if 'needs DELETE permission' in response.content:
+            return True
+
+    # PyPI code path.
+    if response.status_code == 400:
+        filename = package.basefilename
+        # NOTE(sigmavirus24): Old PyPI returns the first message while
+        # Warehouse returns the latter. This papers over the differences.
+        msg = ('A file named "{0}" already exists for'.format(filename),
+               'File already exists')
+        if response.reason.startswith(msg):
+            return True
+
+    # "Other implementations" code path.
+    if response.status_code == 409:
+        return True
+
+    return False
 
 
 def upload(dists, repository, sign, identity, username, password, comment,
@@ -156,7 +177,7 @@ def upload(dists, repository, sign, identity, username, password, comment,
                  ' Aborting...').format(config["repository"],
                                         resp.headers["location"]))
 
-        if skip_upload(resp, skip_existing, package):
+        if skip_existing and ignore_upload_failure(resp, package):
             print(skip_message)
             continue
         utils.check_status_code(resp, verbose)