Skip to content

Commit

Permalink
add --skip-existing to publish
Browse files Browse the repository at this point in the history
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 python-poetry#2254
  • Loading branch information
davidszotten committed Mar 28, 2022
1 parent 3596eba commit d821a43
Show file tree
Hide file tree
Showing 7 changed files with 120 additions and 15 deletions.
1 change: 1 addition & 0 deletions docs/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,7 @@ Should match a repository name set by the [`config`](#config) command.
* `--username (-u)`: The username to access the repository.
* `--password (-p)`: The password to access the repository.
* `--dry-run`: Perform all actions except upload the package.
* `--skip-existing`: Ignore errors from files already existing in the repository.

## config

Expand Down
6 changes: 6 additions & 0 deletions src/poetry/console/commands/publish.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ class PublishCommand(Command):
),
option("build", None, "Build the package before publishing."),
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.
Expand Down Expand Up @@ -82,6 +87,7 @@ def handle(self) -> int | None:
cert,
client_cert,
self.option("dry-run"),
self.option("skip-existing"),
)

return None
2 changes: 2 additions & 0 deletions src/poetry/publishing/publisher.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ def publish(
cert: Path | None = None,
client_cert: Path | None = None,
dry_run: bool = False,
skip_existing: bool = False,
) -> None:
if not repository_name:
url = "https://upload.pypi.org/legacy/"
Expand Down Expand Up @@ -98,4 +99,5 @@ def publish(
cert=cert or get_cert(self._poetry.config, repository_name),
client_cert=resolved_client_cert,
dry_run=dry_run,
skip_existing=skip_existing,
)
38 changes: 35 additions & 3 deletions src/poetry/publishing/uploader.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ def upload(
cert: Path | None = None,
client_cert: Path | None = None,
dry_run: bool = False,
skip_existing: bool = False,
) -> None:
session = self.make_session()

Expand All @@ -126,7 +127,7 @@ def upload(
session.cert = str(client_cert)

try:
self._upload(session, url, dry_run)
self._upload(session, url, dry_run, skip_existing)
finally:
session.close()

Expand Down Expand Up @@ -208,19 +209,24 @@ def post_data(self, file: Path) -> dict[str, Any]:
return data

def _upload(
self, session: requests.Session, url: str, dry_run: bool = False
self,
session: requests.Session,
url: str,
dry_run: bool = False,
skip_existing: bool = False,
) -> None:
for file in self.files:
# TODO: Check existence

self._upload_file(session, url, file, dry_run)
self._upload_file(session, url, file, dry_run, skip_existing)

def _upload_file(
self,
session: requests.Session,
url: str,
file: Path,
dry_run: bool = False,
skip_existing: bool = False,
) -> None:
from cleo.ui.progress_bar import ProgressBar

Expand Down Expand Up @@ -275,6 +281,12 @@ def _upload_file(
elif resp.status_code == 400 and "was ever registered" in resp.text:
self._register(session, url)
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:
resp.raise_for_status()
except (requests.ConnectionError, requests.HTTPError) as e:
Expand Down Expand Up @@ -334,3 +346,23 @@ def _get_type(self, file: Path) -> str:
return "sdist"

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)
)
38 changes: 32 additions & 6 deletions tests/console/commands/test_publish.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def test_publish_with_cert(app_tester: ApplicationTester, mocker: MockerFixture)
app_tester.execute("publish --cert path/to/ca.pem")

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


Expand All @@ -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")
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


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.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

Expand All @@ -103,3 +111,21 @@ def test_publish_dry_run(
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-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
22 changes: 16 additions & 6 deletions tests/publishing/test_publisher.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def test_publish_publishes_to_pypi_by_default(
assert [("foo", "bar")] == uploader_auth.call_args
assert [
("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


Expand Down Expand Up @@ -70,7 +70,7 @@ def test_publish_can_publish_to_given_repository(
assert [("foo", "bar")] == uploader_auth.call_args
assert [
("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
assert "Publishing my-package (1.2.3) to foo" in io.fetch_output()

Expand Down Expand Up @@ -104,7 +104,7 @@ def test_publish_uses_token_if_it_exists(
assert [("__token__", "my-token")] == uploader_auth.call_args
assert [
("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


Expand All @@ -130,7 +130,12 @@ def test_publish_uses_cert(
assert [("foo", "bar")] == uploader_auth.call_args
assert [
("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


Expand All @@ -153,7 +158,12 @@ def test_publish_uses_client_cert(

assert [
("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


Expand All @@ -176,5 +186,5 @@ def test_publish_read_from_environment_variable(
assert [("bar", "baz")] == uploader_auth.call_args
assert [
("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
28 changes: 28 additions & 0 deletions tests/publishing/test_uploader.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,31 @@ def test_uploader_registers_for_appropriate_400_errors(
uploader.upload("https://foo.com")

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)

0 comments on commit d821a43

Please sign in to comment.