From 2cf71388947344d429a94d12e8fad531f5893b4d Mon Sep 17 00:00:00 2001 From: Brian Rutledge Date: Mon, 17 Jan 2022 11:51:51 -0500 Subject: [PATCH 1/7] Log all upload responses with --verbose --- twine/commands/upload.py | 7 +++++++ twine/utils.py | 3 --- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/twine/commands/upload.py b/twine/commands/upload.py index 45b6f2a1..deff3e31 100644 --- a/twine/commands/upload.py +++ b/twine/commands/upload.py @@ -140,6 +140,13 @@ def upload(upload_settings: settings.Settings, dists: List[str]) -> None: resp = repository.upload(package) + if upload_settings.verbose: + logger.info( + f"Received {resp.status_code} response from {resp.url}: {resp.reason}" + ) + if resp.text: + logger.info(f"Response text:\n{resp.text}") + # Bug 92. If we get a redirect we should abort because something seems # funky. The behaviour is not well defined and redirects being issued # by PyPI should never happen in reality. This should catch malicious diff --git a/twine/utils.py b/twine/utils.py index 0aa5c75a..d9914078 100644 --- a/twine/utils.py +++ b/twine/utils.py @@ -204,9 +204,6 @@ def check_status_code(response: requests.Response, verbose: bool) -> None: "Retry with the --verbose option for more details." ) - if response.text: - logger.info("Content received from server:\n{}".format(response.text)) - raise err From b458254ee7c13e576982abcff34756da7c7103e0 Mon Sep 17 00:00:00 2001 From: Brian Rutledge Date: Tue, 18 Jan 2022 19:37:36 -0500 Subject: [PATCH 2/7] Use requests_toolbelt to log request/response --- twine/commands/upload.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/twine/commands/upload.py b/twine/commands/upload.py index deff3e31..e6928c19 100644 --- a/twine/commands/upload.py +++ b/twine/commands/upload.py @@ -18,6 +18,7 @@ from typing import Dict, List, cast import requests +from requests_toolbelt.utils import dump from twine import commands from twine import exceptions @@ -141,11 +142,7 @@ def upload(upload_settings: settings.Settings, dists: List[str]) -> None: resp = repository.upload(package) if upload_settings.verbose: - logger.info( - f"Received {resp.status_code} response from {resp.url}: {resp.reason}" - ) - if resp.text: - logger.info(f"Response text:\n{resp.text}") + logger.info(dump.dump_all(resp).decode("utf-8")) # Bug 92. If we get a redirect we should abort because something seems # funky. The behaviour is not well defined and redirects being issued From b00659a9decbc1085caf4772e8300fbfd237b6f3 Mon Sep 17 00:00:00 2001 From: Brian Rutledge Date: Sat, 22 Jan 2022 06:51:30 -0500 Subject: [PATCH 3/7] Revert "Use requests_toolbelt to log request/response" This reverts commit b458254ee7c13e576982abcff34756da7c7103e0. --- twine/commands/upload.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/twine/commands/upload.py b/twine/commands/upload.py index e6928c19..deff3e31 100644 --- a/twine/commands/upload.py +++ b/twine/commands/upload.py @@ -18,7 +18,6 @@ from typing import Dict, List, cast import requests -from requests_toolbelt.utils import dump from twine import commands from twine import exceptions @@ -142,7 +141,11 @@ def upload(upload_settings: settings.Settings, dists: List[str]) -> None: resp = repository.upload(package) if upload_settings.verbose: - logger.info(dump.dump_all(resp).decode("utf-8")) + logger.info( + f"Received {resp.status_code} response from {resp.url}: {resp.reason}" + ) + if resp.text: + logger.info(f"Response text:\n{resp.text}") # Bug 92. If we get a redirect we should abort because something seems # funky. The behaviour is not well defined and redirects being issued From 788ce712850602bf83a751b35209c5a177550145 Mon Sep 17 00:00:00 2001 From: Brian Rutledge Date: Sat, 22 Jan 2022 17:19:30 -0500 Subject: [PATCH 4/7] Log request details --- twine/commands/upload.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/twine/commands/upload.py b/twine/commands/upload.py index deff3e31..b967d02d 100644 --- a/twine/commands/upload.py +++ b/twine/commands/upload.py @@ -141,9 +141,8 @@ def upload(upload_settings: settings.Settings, dists: List[str]) -> None: resp = repository.upload(package) if upload_settings.verbose: - logger.info( - f"Received {resp.status_code} response from {resp.url}: {resp.reason}" - ) + logger.info(f"Request: {resp.request.method} {resp.request.url}") + logger.info(f"Response from {resp.url}:\n{resp.status_code} {resp.reason}") if resp.text: logger.info(f"Response text:\n{resp.text}") From d3485790c3904af7b94779e863c3494173f6105c Mon Sep 17 00:00:00 2001 From: Brian Rutledge Date: Sat, 22 Jan 2022 18:07:00 -0500 Subject: [PATCH 5/7] Fix failing tests --- tests/test_upload.py | 17 ++++++++++++++--- tests/test_utils.py | 5 +---- twine/commands/upload.py | 9 +++------ 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/tests/test_upload.py b/tests/test_upload.py index e6ff45e1..55b8b6e4 100644 --- a/tests/test_upload.py +++ b/tests/test_upload.py @@ -32,7 +32,12 @@ def stub_response(): """Mock successful upload of a package.""" return pretend.stub( - is_redirect=False, status_code=201, raise_for_status=lambda: None + is_redirect=False, + url="https://test.pypi.org/legacy/", + status_code=200, + reason="OK", + text=None, + raise_for_status=lambda: None, ) @@ -186,7 +191,8 @@ def test_exception_for_http_status(verbose, upload_settings, stub_response, caps stub_response.is_redirect = False stub_response.status_code = 403 - stub_response.text = "Invalid or non-existent authentication information" + stub_response.reason = "Invalid or non-existent authentication information" + stub_response.text = stub_response.reason stub_response.raise_for_status = pretend.raiser(requests.HTTPError) with pytest.raises(requests.HTTPError): @@ -288,8 +294,11 @@ def test_exception_for_redirect( stub_response = pretend.stub( is_redirect=True, + url=redirect_url, status_code=301, headers={"location": redirect_url}, + reason="Redirect", + text="", ) stub_repository = pretend.stub( @@ -323,7 +332,9 @@ def test_prints_skip_message_for_response( ): upload_settings.skip_existing = True - stub_response.status_code = 409 + stub_response.status_code = 400 + stub_response.reason = "File already exists" + stub_response.text = stub_response.reason # Do the upload, triggering the error response stub_repository.package_is_uploaded = lambda package: False diff --git a/tests/test_utils.py b/tests/test_utils.py index 07965d54..7eaebdae 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -282,10 +282,7 @@ def test_check_status_code_for_missing_status_code( captured = capsys.readouterr() - if verbose: - assert captured.out.count("Content received from server:\nForbidden\n") == 1 - else: - assert captured.out.count("--verbose option") == 1 + assert captured.out.count("--verbose option") == 0 if verbose else 1 @pytest.mark.parametrize( diff --git a/twine/commands/upload.py b/twine/commands/upload.py index b967d02d..a460e664 100644 --- a/twine/commands/upload.py +++ b/twine/commands/upload.py @@ -139,12 +139,9 @@ def upload(upload_settings: settings.Settings, dists: List[str]) -> None: continue resp = repository.upload(package) - - if upload_settings.verbose: - logger.info(f"Request: {resp.request.method} {resp.request.url}") - logger.info(f"Response from {resp.url}:\n{resp.status_code} {resp.reason}") - if resp.text: - logger.info(f"Response text:\n{resp.text}") + logger.info(f"Response from {resp.url}:\n{resp.status_code} {resp.reason}") + if resp.text: + logger.info(resp.text) # Bug 92. If we get a redirect we should abort because something seems # funky. The behaviour is not well defined and redirects being issued From c800a2185b786eb118093d7b6e2c67b61d136f82 Mon Sep 17 00:00:00 2001 From: Brian Rutledge Date: Sat, 22 Jan 2022 18:17:10 -0500 Subject: [PATCH 6/7] Add new test --- tests/test_upload.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/test_upload.py b/tests/test_upload.py index 55b8b6e4..8836fef1 100644 --- a/tests/test_upload.py +++ b/tests/test_upload.py @@ -143,6 +143,25 @@ def test_print_packages_if_verbose(upload_settings, capsys): assert captured.out.count(f"{filename} ({size})") == 1 +def test_print_response_if_verbose(upload_settings, stub_response, capsys): + """Print details about the response from the repostiry.""" + upload_settings.verbose = True + + result = upload.upload( + upload_settings, + [helpers.WHEEL_FIXTURE, helpers.SDIST_FIXTURE], + ) + assert result is None + + captured = capsys.readouterr() + response_log = ( + f"Response from {stub_response.url}:\n" + f"{stub_response.status_code} {stub_response.reason}" + ) + + assert captured.out.count(response_log) == 2 + + def test_success_with_pre_signed_distribution(upload_settings, stub_repository): """Add GPG signature provided by user to uploaded package.""" # Upload a pre-signed distribution From fbf4b732423a704da4fc5d620905b47133bc4db8 Mon Sep 17 00:00:00 2001 From: Brian Rutledge Date: Mon, 24 Jan 2022 05:51:32 -0500 Subject: [PATCH 7/7] Add changelog entry --- changelog/859.feature.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/859.feature.rst diff --git a/changelog/859.feature.rst b/changelog/859.feature.rst new file mode 100644 index 00000000..223361b5 --- /dev/null +++ b/changelog/859.feature.rst @@ -0,0 +1 @@ +Log all upload responses with ``--verbose``.