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 #2254
  • Loading branch information
davidszotten committed Aug 17, 2020
1 parent 4bc181b commit a530706
Show file tree
Hide file tree
Showing 7 changed files with 106 additions and 27 deletions.
1 change: 1 addition & 0 deletions docs/docs/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,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 poetry/console/commands/publish.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,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 @@ -81,4 +86,5 @@ def handle(self):
cert,
client_cert,
self.option("dry-run"),
self.option("skip-existing"),
)
4 changes: 3 additions & 1 deletion poetry/publishing/publisher.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ def publish(
cert=None,
client_cert=None,
dry_run=False,
): # type: (Optional[str], Optional[str], Optional[str], Optional[Path], Optional[Path], Optional[bool]) -> None
skip_existing=False,
): # type: (Optional[str], Optional[str], Optional[str], Optional[Path], Optional[Path], Optional[bool], Optional[bool]) -> None
if not repository_name:
url = "https://upload.pypi.org/legacy/"
repository_name = "pypi"
Expand Down Expand Up @@ -95,4 +96,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,
)
69 changes: 51 additions & 18 deletions poetry/publishing/uploader.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,32 @@ def __init__(self, error): # type: (Union[ConnectionError, HTTPError]) -> None
super(UploadError, self).__init__(message)


def _ignore_existing(
response, skip_existing
): # type: (requests.Response, bool) -> bool
# based on https://github.com/pypa/twine/blob/62cec5b77037345b4fe1a85bbc6b104440799496/twine/commands/upload.py#L32
if not skip_existing:
return False

status = response.status_code
reason = getattr(response, "reason", "").lower()
text = getattr(response, "text", "").lower()

# NOTE(sigmavirus24): PyPI presently returns a 400 status code with the
# error message in the reason attribute. Other implementations return a
# 403 or 409 status code.
return (
# pypiserver (https://pypi.org/project/pypiserver)
status == 409
# PyPI / TestPyPI
or (status == 400 and "already exist" in reason)
# Nexus Repository OSS (https://www.sonatype.com/nexus-repository-oss)
or (status == 400 and "updating asset" in reason)
# Artifactory (https://jfrog.com/artifactory/)
or (status == 403 and "overwrite artifact" in text)
)


class Uploader:
def __init__(self, poetry, io):
self._poetry = poetry
Expand Down Expand Up @@ -105,8 +131,8 @@ def is_authenticated(self):
return self._username is not None and self._password is not None

def upload(
self, url, cert=None, client_cert=None, dry_run=False
): # type: (str, Optional[Path], Optional[Path], bool) -> None
self, url, cert=None, client_cert=None, dry_run=False, skip_existing=False,
): # type: (str, Optional[Path], Optional[Path], bool, bool) -> None
session = self.make_session()

if cert:
Expand All @@ -116,7 +142,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 @@ -199,10 +225,10 @@ def post_data(self, file): # type: (Path) -> Dict[str, Any]
return data

def _upload(
self, session, url, dry_run=False
): # type: (requests.Session, str, Optional[bool]) -> None
self, session, url, dry_run=False, skip_existing=False,
): # type: (requests.Session, str, Optional[bool], Optional[bool]) -> None
try:
self._do_upload(session, url, dry_run)
self._do_upload(session, url, dry_run, skip_existing)
except HTTPError as e:
if (
e.response.status_code == 400
Expand All @@ -216,19 +242,19 @@ def _upload(
raise UploadError(e)

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

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

if not dry_run:
if not dry_run and not _ignore_existing(resp, skip_existing):
resp.raise_for_status()

def _upload_file(
self, session, url, file, dry_run=False
): # type: (requests.Session, str, Path, Optional[bool]) -> requests.Response
self, session, url, file, dry_run=False, skip_existing=False,
): # type: (requests.Session, str, Path, Optional[bool], Optional[bool]) -> requests.Response
data = self.post_data(file)
data.update(
{
Expand Down Expand Up @@ -271,16 +297,23 @@ def _upload_file(
file.name
)
)
bar.finish()
except (requests.ConnectionError, requests.HTTPError) as e:
if self._io.output.supports_ansi():
self._io.overwrite(
" - Uploading <c1>{0}</c1> <error>{1}</>".format(
file.name, "FAILED"
)
bar.set_format(
" - Uploading <c1>{0}</c1> <error>{1}</>".format(
file.name, "FAILED"
)
)
raise UploadError(e)
else:
if _ignore_existing(resp, skip_existing):
bar.set_format(
" - Uploading <c1>{0}</c1> <warning>{1}</>".format(
file.name, "File exists. Skipping"
),
)

finally:
bar.finish()
self._io.write_line("")

return resp
Expand Down
23 changes: 21 additions & 2 deletions tests/console/commands/test_publish.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def test_publish_with_cert(app_tester, mocker):
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 @@ -88,7 +88,7 @@ def test_publish_with_client_cert(app_tester, mocker):

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


Expand All @@ -107,3 +107,22 @@ def test_publish_dry_run(app_tester, http):
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_publish_skip_existing(app_tester, http):
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 0 == exit_code

output = app_tester.io.fetch_output()
error = app_tester.io.fetch_error()

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
22 changes: 16 additions & 6 deletions tests/publishing/test_publisher.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def test_publish_publishes_to_pypi_by_default(fixture_dir, mocker, config):
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 All @@ -45,7 +45,7 @@ def test_publish_can_publish_to_given_repository(fixture_dir, mocker, config):
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


Expand Down Expand Up @@ -74,7 +74,7 @@ def test_publish_uses_token_if_it_exists(fixture_dir, mocker, config):
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 @@ -98,7 +98,12 @@ def test_publish_uses_cert(fixture_dir, mocker, config):
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 @@ -119,7 +124,12 @@ def test_publish_uses_client_cert(fixture_dir, mocker, config):

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 @@ -137,5 +147,5 @@ def test_publish_read_from_environment_variable(fixture_dir, environ, mocker, co
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
8 changes: 8 additions & 0 deletions tests/publishing/test_uploader.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,11 @@ def test_uploader_registers_for_appropriate_400_errors(mocker, http):
uploader.upload("https://foo.com")

assert 1 == register.call_count


def test_uploader_skips_existing(http):
http.register_uri(http.POST, "https://foo.com", status=409, body="Conflict")
uploader = Uploader(Factory().create_poetry(project("simple_project")), NullIO())

# should not raise
uploader.upload("https://foo.com", skip_existing=True)

0 comments on commit a530706

Please sign in to comment.