Skip to content

Commit

Permalink
Log all upload responses with --verbose (#859)
Browse files Browse the repository at this point in the history
* Log all upload responses with --verbose

* Use requests_toolbelt to log request/response

* Revert "Use requests_toolbelt to log request/response"

This reverts commit b458254.

* Log request details

* Fix failing tests

* Add new test

* Add changelog entry
  • Loading branch information
bhrutledge authored Jan 24, 2022
1 parent a60c565 commit cf9295f
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 10 deletions.
1 change: 1 addition & 0 deletions changelog/859.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Log all upload responses with ``--verbose``.
36 changes: 33 additions & 3 deletions tests/test_upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)


Expand Down Expand Up @@ -138,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
Expand Down Expand Up @@ -186,7 +210,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):
Expand Down Expand Up @@ -288,8 +313,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(
Expand Down Expand Up @@ -323,7 +351,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
Expand Down
5 changes: 1 addition & 4 deletions tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
3 changes: 3 additions & 0 deletions twine/commands/upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,9 @@ def upload(upload_settings: settings.Settings, dists: List[str]) -> None:
continue

resp = repository.upload(package)
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
Expand Down
3 changes: 0 additions & 3 deletions twine/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down

0 comments on commit cf9295f

Please sign in to comment.