add --skip-existing to publish (#2812)

* dry_run is never None

* add --skip-existing to publish

add `--skip-existing` option to publish command to continue if an upload
fails due to the file already existing. this is helpful in e.g. ci
settings. resolves #2254

docs/cli.md

@@ -482,6 +482,7 @@ Should match a repository name set by the [`config`](#config) command.
 * `--username (-u)`: The username to access the repository.
 * `--username (-u)`: The username to access the repository.
 * `--password (-p)`: The password to access the repository.
 * `--password (-p)`: The password to access the repository.
 * `--dry-run`: Perform all actions except upload the package.
 * `--dry-run`: Perform all actions except upload the package.
+* `--skip-existing`: Ignore errors from files already existing in the repository.
 
 
 ## config
 ## config
 
 

src/poetry/console/commands/publish.py

@@ -29,6 +29,11 @@ class PublishCommand(Command):
         ),
         ),
         option("build", None, "Build the package before publishing."),
         option("build", None, "Build the package before publishing."),
         option("dry-run", None, "Perform all actions except upload the package."),
         option("dry-run", None, "Perform all actions except upload the package."),
+        option(
+            "skip-existing",
+            None,
+            "Ignore errors from files already existing in the repository.",
+        ),
     ]
     ]
 
 
     help = """The publish command builds and uploads the package to a remote repository.
     help = """The publish command builds and uploads the package to a remote repository.
@@ -82,6 +87,7 @@ the config command.
             cert,
             cert,
             client_cert,
             client_cert,
             self.option("dry-run"),
             self.option("dry-run"),
+            self.option("skip-existing"),
         )
         )
 
 
         return None
         return None

src/poetry/publishing/publisher.py

@@ -45,6 +45,7 @@ class Publisher:
         cert: Path | None = None,
         cert: Path | None = None,
         client_cert: Path | None = None,
         client_cert: Path | None = None,
         dry_run: bool = False,
         dry_run: bool = False,
+        skip_existing: bool = False,
     ) -> None:
     ) -> None:
         if not repository_name:
         if not repository_name:
             url = "https://upload.pypi.org/legacy/"
             url = "https://upload.pypi.org/legacy/"
@@ -98,4 +99,5 @@ class Publisher:
             cert=cert or get_cert(self._poetry.config, repository_name),
             cert=cert or get_cert(self._poetry.config, repository_name),
             client_cert=resolved_client_cert,
             client_cert=resolved_client_cert,
             dry_run=dry_run,
             dry_run=dry_run,
+            skip_existing=skip_existing,
         )
         )

src/poetry/publishing/uploader.py

@@ -116,6 +116,7 @@ class Uploader:
         cert: Path | None = None,
         cert: Path | None = None,
         client_cert: Path | None = None,
         client_cert: Path | None = None,
         dry_run: bool = False,
         dry_run: bool = False,
+        skip_existing: bool = False,
     ) -> None:
     ) -> None:
         session = self.make_session()
         session = self.make_session()
 
 
@@ -126,7 +127,7 @@ class Uploader:
             session.cert = str(client_cert)
             session.cert = str(client_cert)
 
 
         try:
         try:
-            self._upload(session, url, dry_run)
+            self._upload(session, url, dry_run, skip_existing)
         finally:
         finally:
             session.close()
             session.close()
 
 
@@ -208,19 +209,24 @@ class Uploader:
         return data
         return data
 
 
     def _upload(
     def _upload(
-        self, session: requests.Session, url: str, dry_run: bool | None = False
+        self,
+        session: requests.Session,
+        url: str,
+        dry_run: bool = False,
+        skip_existing: bool = False,
     ) -> None:
     ) -> None:
         for file in self.files:
         for file in self.files:
             # TODO: Check existence
             # TODO: Check existence
 
 
-            self._upload_file(session, url, file, dry_run)
+            self._upload_file(session, url, file, dry_run, skip_existing)
 
 
     def _upload_file(
     def _upload_file(
         self,
         self,
         session: requests.Session,
         session: requests.Session,
         url: str,
         url: str,
         file: Path,
         file: Path,
-        dry_run: bool | None = False,
+        dry_run: bool = False,
+        skip_existing: bool = False,
     ) -> None:
     ) -> None:
         from cleo.ui.progress_bar import ProgressBar
         from cleo.ui.progress_bar import ProgressBar
 
 
@@ -275,6 +281,12 @@ class Uploader:
                 elif resp.status_code == 400 and "was ever registered" in resp.text:
                 elif resp.status_code == 400 and "was ever registered" in resp.text:
                     self._register(session, url)
                     self._register(session, url)
                     resp.raise_for_status()
                     resp.raise_for_status()
+                elif skip_existing and self._is_file_exists_error(resp):
+                    bar.set_format(
+                        f" - Uploading <c1>{file.name}</c1> <warning>File exists."
+                        " Skipping</>"
+                    )
+                    bar.display()
                 else:
                 else:
                     resp.raise_for_status()
                     resp.raise_for_status()
             except (requests.ConnectionError, requests.HTTPError) as e:
             except (requests.ConnectionError, requests.HTTPError) as e:
@@ -334,3 +346,23 @@ class Uploader:
             return "sdist"
             return "sdist"
 
 
         raise ValueError("Unknown distribution format " + "".join(exts))
         raise ValueError("Unknown distribution format " + "".join(exts))
+
+    def _is_file_exists_error(self, response: requests.Response) -> bool:
+        # based on https://github.com/pypa/twine/blob/a6dd69c79f7b5abfb79022092a5d3776a499e31b/twine/commands/upload.py#L32  # noqa: E501
+        status = response.status_code
+        reason = response.reason.lower()
+        text = response.text.lower()
+        reason_and_text = reason + text
+
+        return (
+            # pypiserver (https://pypi.org/project/pypiserver)
+            status == 409
+            # PyPI / TestPyPI / GCP Artifact Registry
+            or (status == 400 and "already exist" in reason_and_text)
+            # Nexus Repository OSS (https://www.sonatype.com/nexus-repository-oss)
+            or (status == 400 and "updating asset" in reason_and_text)
+            # Artifactory (https://jfrog.com/artifactory/)
+            or (status == 403 and "overwrite artifact" in reason_and_text)
+            # Gitlab Enterprise Edition (https://about.gitlab.com)
+            or (status == 400 and "already been taken" in reason_and_text)
+        )

tests/console/commands/test_publish.py

@@ -73,7 +73,7 @@ def test_publish_with_cert(app_tester: ApplicationTester, mocker: MockerFixture)
     app_tester.execute("publish --cert path/to/ca.pem")
     app_tester.execute("publish --cert path/to/ca.pem")
 
 
     assert [
     assert [
-        (None, None, None, Path("path/to/ca.pem"), None, False)
+        (None, None, None, Path("path/to/ca.pem"), None, False, False)
     ] == publisher_publish.call_args
     ] == publisher_publish.call_args
 
 
 
 
@@ -82,18 +82,26 @@ def test_publish_with_client_cert(app_tester: ApplicationTester, mocker: MockerF
 
 
     app_tester.execute("publish --client-cert path/to/client.pem")
     app_tester.execute("publish --client-cert path/to/client.pem")
     assert [
     assert [
-        (None, None, None, None, Path("path/to/client.pem"), False)
+        (None, None, None, None, Path("path/to/client.pem"), False, False)
     ] == publisher_publish.call_args
     ] == publisher_publish.call_args
 
 
 
 
-def test_publish_dry_run(
-    app_tester: ApplicationTester, http: type[httpretty.httpretty]
+@pytest.mark.parametrize(
+    "options",
+    [
+        "--dry-run",
+        "--skip-existing",
+        "--dry-run --skip-existing",
+    ],
+)
+def test_publish_dry_run_skip_existing(
+    app_tester: ApplicationTester, http: type[httpretty.httpretty], options: str
 ):
 ):
     http.register_uri(
     http.register_uri(
-        http.POST, "https://upload.pypi.org/legacy/", status=403, body="Forbidden"
+        http.POST, "https://upload.pypi.org/legacy/", status=409, body="Conflict"
     )
     )
 
 
-    exit_code = app_tester.execute("publish --dry-run --username foo --password bar")
+    exit_code = app_tester.execute(f"publish {options} --username foo --password bar")
 
 
     assert exit_code == 0
     assert exit_code == 0
 
 
@@ -103,3 +111,21 @@ def test_publish_dry_run(
     assert "Publishing simple-project (1.2.3) to PyPI" in output
     assert "Publishing simple-project (1.2.3) to PyPI" in output
     assert "- Uploading simple-project-1.2.3.tar.gz" in error
     assert "- Uploading simple-project-1.2.3.tar.gz" in error
     assert "- Uploading simple_project-1.2.3-py2.py3-none-any.whl" in error
     assert "- Uploading simple_project-1.2.3-py2.py3-none-any.whl" in error
+
+
+def test_skip_existing_output(
+    app_tester: ApplicationTester, http: type[httpretty.httpretty]
+):
+
+    http.register_uri(
+        http.POST, "https://upload.pypi.org/legacy/", status=409, body="Conflict"
+    )
+
+    exit_code = app_tester.execute(
+        "publish --skip-existing --username foo --password bar"
+    )
+
+    assert exit_code == 0
+
+    error = app_tester.io.fetch_error()
+    assert "- Uploading simple-project-1.2.3.tar.gz File exists. Skipping" in error

tests/publishing/test_publisher.py

@@ -38,7 +38,7 @@ def test_publish_publishes_to_pypi_by_default(
     assert [("foo", "bar")] == uploader_auth.call_args
     assert [("foo", "bar")] == uploader_auth.call_args
     assert [
     assert [
         ("https://upload.pypi.org/legacy/",),
         ("https://upload.pypi.org/legacy/",),
-        {"cert": None, "client_cert": None, "dry_run": False},
+        {"cert": None, "client_cert": None, "dry_run": False, "skip_existing": False},
     ] == uploader_upload.call_args
     ] == uploader_upload.call_args
 
 
 
 
@@ -70,7 +70,7 @@ def test_publish_can_publish_to_given_repository(
     assert [("foo", "bar")] == uploader_auth.call_args
     assert [("foo", "bar")] == uploader_auth.call_args
     assert [
     assert [
         ("http://foo.bar",),
         ("http://foo.bar",),
-        {"cert": None, "client_cert": None, "dry_run": False},
+        {"cert": None, "client_cert": None, "dry_run": False, "skip_existing": False},
     ] == uploader_upload.call_args
     ] == uploader_upload.call_args
     assert "Publishing my-package (1.2.3) to foo" in io.fetch_output()
     assert "Publishing my-package (1.2.3) to foo" in io.fetch_output()
 
 
@@ -104,7 +104,7 @@ def test_publish_uses_token_if_it_exists(
     assert [("__token__", "my-token")] == uploader_auth.call_args
     assert [("__token__", "my-token")] == uploader_auth.call_args
     assert [
     assert [
         ("https://upload.pypi.org/legacy/",),
         ("https://upload.pypi.org/legacy/",),
-        {"cert": None, "client_cert": None, "dry_run": False},
+        {"cert": None, "client_cert": None, "dry_run": False, "skip_existing": False},
     ] == uploader_upload.call_args
     ] == uploader_upload.call_args
 
 
 
 
@@ -130,7 +130,12 @@ def test_publish_uses_cert(
     assert [("foo", "bar")] == uploader_auth.call_args
     assert [("foo", "bar")] == uploader_auth.call_args
     assert [
     assert [
         ("https://foo.bar",),
         ("https://foo.bar",),
-        {"cert": Path(cert), "client_cert": None, "dry_run": False},
+        {
+            "cert": Path(cert),
+            "client_cert": None,
+            "dry_run": False,
+            "skip_existing": False,
+        },
     ] == uploader_upload.call_args
     ] == uploader_upload.call_args
 
 
 
 
@@ -153,7 +158,12 @@ def test_publish_uses_client_cert(
 
 
     assert [
     assert [
         ("https://foo.bar",),
         ("https://foo.bar",),
-        {"cert": None, "client_cert": Path(client_cert), "dry_run": False},
+        {
+            "cert": None,
+            "client_cert": Path(client_cert),
+            "dry_run": False,
+            "skip_existing": False,
+        },
     ] == uploader_upload.call_args
     ] == uploader_upload.call_args
 
 
 
 
@@ -176,5 +186,5 @@ def test_publish_read_from_environment_variable(
     assert [("bar", "baz")] == uploader_auth.call_args
     assert [("bar", "baz")] == uploader_auth.call_args
     assert [
     assert [
         ("https://foo.bar",),
         ("https://foo.bar",),
-        {"cert": None, "client_cert": None, "dry_run": False},
+        {"cert": None, "client_cert": None, "dry_run": False, "skip_existing": False},
     ] == uploader_upload.call_args
     ] == uploader_upload.call_args

tests/publishing/test_uploader.py

@@ -72,3 +72,31 @@ def test_uploader_registers_for_appropriate_400_errors(
         uploader.upload("https://foo.com")
         uploader.upload("https://foo.com")
 
 
     assert register.call_count == 1
     assert register.call_count == 1
+
+
+@pytest.mark.parametrize(
+    "status, body",
+    [
+        (409, ""),
+        (400, "File already exists"),
+        (400, "Repository does not allow updating assets"),
+        (403, "Not enough permissions to overwrite artifact"),
+        (400, "file name has already been taken"),
+    ],
+)
+def test_uploader_skips_existing(
+    http: type[httpretty.httpretty], uploader: Uploader, status: int, body: str
+):
+    http.register_uri(http.POST, "https://foo.com", status=status, body=body)
+
+    # should not raise
+    uploader.upload("https://foo.com", skip_existing=True)
+
+
+def test_uploader_skip_existing_bubbles_unskippable_errors(
+    http: type[httpretty.httpretty], uploader: Uploader
+):
+    http.register_uri(http.POST, "https://foo.com", status=403, body="Unauthorized")
+
+    with pytest.raises(UploadError):
+        uploader.upload("https://foo.com", skip_existing=True)