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

add --skip-existing to publish #2812

Merged
merged 2 commits into from
Mar 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,
)
40 changes: 36 additions & 4 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 | None = 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 | None = False,
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")
radoering marked this conversation as resolved.
Show resolved Hide resolved

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)