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

Setup basic logging functionality(existing output) with 1 level of verbosity #670

Merged
merged 4 commits into from
Jul 15, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
13 changes: 13 additions & 0 deletions tests/test_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import argparse
import logging
import os.path
import textwrap

Expand Down Expand Up @@ -55,6 +56,18 @@ def test_settings_transforms_repository_config(tmpdir):
assert s.disable_progress_bar is False


@pytest.mark.parametrize("verbose", [True, False])
def test_setup_logging(verbose: bool):
bhrutledge marked this conversation as resolved.
Show resolved Hide resolved
"""Set log level based on verbose field."""
settings._setup_logging(verbose)
bhrutledge marked this conversation as resolved.
Show resolved Hide resolved
logger = logging.getLogger("twine")

if verbose:
assert logger.level == logging.INFO
else:
assert logger.level == logging.WARNING


def test_identity_requires_sign():
"""Raise an exception when user provides identity but doesn't require sigining."""
with pytest.raises(exceptions.InvalidSigningConfiguration):
Expand Down
47 changes: 28 additions & 19 deletions tests/test_upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import logging
import os

import pretend
Expand Down Expand Up @@ -54,7 +55,7 @@ def upload_settings(make_settings, stub_repository):
return upload_settings


def test_make_package_pre_signed_dist(upload_settings, capsys):
def test_make_package_pre_signed_dist(upload_settings, caplog):
"""Create a PackageFile and print path, size, and user-provided signature."""
filename = helpers.WHEEL_FIXTURE
expected_size = "15.4 KB"
Expand All @@ -63,25 +64,27 @@ def test_make_package_pre_signed_dist(upload_settings, capsys):

upload_settings.sign = True
upload_settings.verbose = True
caplog.set_level(logging.INFO, logger="twine")
bhrutledge marked this conversation as resolved.
Show resolved Hide resolved

package = upload._make_package(filename, signatures, upload_settings)

assert package.filename == filename
assert package.gpg_signature is not None

captured = capsys.readouterr()
assert captured.out.count(f"{filename} ({expected_size})") == 1
assert captured.out.count(f"Signed with {signed_filename}") == 1
captured = caplog.text
assert captured.count(f"{filename} ({expected_size})") == 1
assert captured.count(f"Signed with {signed_filename}") == 1


def test_make_package_unsigned_dist(upload_settings, monkeypatch, capsys):
def test_make_package_unsigned_dist(upload_settings, monkeypatch, caplog):
"""Create a PackageFile and print path, size, and Twine-generated signature."""
filename = helpers.NEW_WHEEL_FIXTURE
expected_size = "21.9 KB"
signatures = {}

upload_settings.sign = True
upload_settings.verbose = True
caplog.set_level(logging.INFO, logger="twine")

def stub_sign(package, *_):
package.gpg_signature = (package.signed_basefilename, b"signature")
Expand All @@ -93,9 +96,9 @@ def stub_sign(package, *_):
assert package.filename == filename
assert package.gpg_signature is not None

captured = capsys.readouterr()
assert captured.out.count(f"{filename} ({expected_size})") == 1
assert captured.out.count(f"Signed with {package.signed_filename}") == 1
captured = caplog.text
assert captured.count(f"{filename} ({expected_size})") == 1
assert captured.count(f"Signed with {package.signed_filename}") == 1


def test_successs_prints_release_urls(upload_settings, stub_repository, capsys):
Expand All @@ -118,7 +121,7 @@ def test_successs_prints_release_urls(upload_settings, stub_repository, capsys):
assert captured.out.count(NEW_RELEASE_URL) == 1


def test_print_packages_if_verbose(upload_settings, capsys):
def test_print_packages_if_verbose(upload_settings, caplog):
"""Print the path and file size of each distribution attempting to be uploaded."""
dists_to_upload = {
helpers.WHEEL_FIXTURE: "15.4 KB",
Expand All @@ -128,14 +131,16 @@ def test_print_packages_if_verbose(upload_settings, capsys):
}

upload_settings.verbose = True
caplog.set_level(logging.INFO, logger="twine")

result = upload.upload(upload_settings, dists_to_upload)

result = upload.upload(upload_settings, dists_to_upload.keys())
assert result is None

captured = capsys.readouterr()
captured = caplog.text

for filename, size in dists_to_upload.items():
assert captured.out.count(f"{filename} ({size})") == 1
assert captured.count(f"{filename} ({size})") == 1


def test_success_with_pre_signed_distribution(upload_settings, stub_repository):
Expand Down Expand Up @@ -181,8 +186,12 @@ def test_success_when_gpg_is_run(upload_settings, stub_repository, monkeypatch):


@pytest.mark.parametrize("verbose", [False, True])
def test_exception_for_http_status(verbose, upload_settings, stub_response, capsys):
def test_exception_for_http_status(verbose, upload_settings, stub_response, caplog):
upload_settings.verbose = verbose
if verbose:
caplog.set_level(logging.INFO, logger="twine")
else:
caplog.set_level(logging.WARNING, logger="twine")
bhrutledge marked this conversation as resolved.
Show resolved Hide resolved

stub_response.is_redirect = False
stub_response.status_code = 403
Expand All @@ -192,15 +201,15 @@ def test_exception_for_http_status(verbose, upload_settings, stub_response, caps
with pytest.raises(requests.HTTPError):
upload.upload(upload_settings, [helpers.WHEEL_FIXTURE])

captured = capsys.readouterr()
assert RELEASE_URL not in captured.out
captured = caplog.text
assert RELEASE_URL not in captured

if verbose:
assert stub_response.text in captured.out
assert "--verbose" not in captured.out
assert stub_response.text in captured
assert "--verbose" not in captured
else:
assert stub_response.text not in captured.out
assert "--verbose" in captured.out
assert stub_response.text not in captured
assert "--verbose" in captured


def test_get_config_old_format(make_settings, pypirc):
Expand Down
16 changes: 10 additions & 6 deletions tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

import logging
import os.path
import textwrap

Expand Down Expand Up @@ -273,7 +273,7 @@ def test_check_status_code_for_deprecated_pypi_url(repo_url):
@pytest.mark.parametrize(
"repo_url", ["https://pypi.python.org", "https://testpypi.python.org"],
)
def test_check_status_code_for_missing_status_code(capsys, repo_url):
def test_check_status_code_for_missing_status_code(caplog, repo_url):
"""Print HTTP errors based on verbosity level."""
response = pretend.stub(
status_code=403,
Expand All @@ -282,18 +282,22 @@ def test_check_status_code_for_missing_status_code(capsys, repo_url):
text="Forbidden",
)

caplog.set_level(logging.INFO, logger="twine")

with pytest.raises(requests.HTTPError):
utils.check_status_code(response, True)

# Different messages are printed based on the verbose level
captured = capsys.readouterr()
assert captured.out == "Content received from server:\nForbidden\n"
captured = caplog.text
assert "Content received from server:\nForbidden\n" in captured

caplog.set_level(logging.WARNING, logger="twine")

with pytest.raises(requests.HTTPError):
utils.check_status_code(response, False)

captured = capsys.readouterr()
assert captured.out == "NOTE: Try --verbose to see response content.\n"
captured = caplog.text
assert "NOTE: Try --verbose to see response content.\n" in captured


@pytest.mark.parametrize(
Expand Down
3 changes: 1 addition & 2 deletions twine/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,10 @@
)

__copyright__ = "Copyright 2019 Donald Stufft and individual contributors"

import sys

if sys.version_info[:2] >= (3, 8):
import importlib.metadata as importlib_metadata
from importlib import metadata as importlib_metadata
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What prompted this change?

Copy link
Contributor Author

@VikramJayanthi17 VikramJayanthi17 Jul 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tox -e format actually did this automatically. Will change it back.

Edit:

It fails to pass the linting test if I change this back. Any advice on how to proceed?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right, isort just had a major version upgrade. I restored this in 8eabc53. I'll plan to followup up with some isort tweaking in a separate PR.

else:
import importlib_metadata

Expand Down
12 changes: 7 additions & 5 deletions twine/commands/upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import argparse
import logging
import os.path
from typing import Dict
from typing import List
Expand All @@ -25,6 +26,8 @@
from twine import settings
from twine import utils

logger = logging.getLogger("twine")
bhrutledge marked this conversation as resolved.
Show resolved Hide resolved


def skip_upload(
response: requests.Response, skip_existing: bool, package: package_file.PackageFile
Expand Down Expand Up @@ -63,11 +66,10 @@ def _make_package(
elif upload_settings.sign:
package.sign(upload_settings.sign_with, upload_settings.identity)

if upload_settings.verbose:
file_size = utils.get_file_size(package.filename)
print(f" {package.filename} ({file_size})")
if package.gpg_signature:
print(f" Signed with {package.signed_filename}")
file_size = utils.get_file_size(package.filename)
logger.info(f" {package.filename} ({file_size})")
if package.gpg_signature:
logger.info(f" Signed with {package.signed_filename}")

return package

Expand Down
13 changes: 13 additions & 0 deletions twine/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import argparse
import logging
import sys
from typing import Any
from typing import Optional
from typing import cast
Expand All @@ -23,6 +25,16 @@
from twine import utils


def _setup_logging(verbose: bool) -> None:
"""Initialize a logger based on verbosity available throughout twine."""
bhrutledge marked this conversation as resolved.
Show resolved Hide resolved
logger = logging.getLogger("twine")
bhrutledge marked this conversation as resolved.
Show resolved Hide resolved
log_level = logging.INFO if verbose else logging.WARNING
handler = logging.StreamHandler(sys.stdout)
handler.setLevel(log_level)
logger.addHandler(handler)
logger.setLevel(log_level)
bhrutledge marked this conversation as resolved.
Show resolved Hide resolved


class Settings:
"""Object that manages the configuration for Twine.

Expand Down Expand Up @@ -121,6 +133,7 @@ def __init__(
self.config_file = config_file
self.comment = comment
self.verbose = verbose
_setup_logging(verbose)
self.disable_progress_bar = disable_progress_bar
self.skip_existing = skip_existing
self._handle_repository_options(
Expand Down
7 changes: 5 additions & 2 deletions twine/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import collections
import configparser
import functools
import logging
import os
import os.path
from typing import Any
Expand Down Expand Up @@ -45,6 +46,8 @@
# get_userpass_value.
RepositoryConfig = Dict[str, Optional[str]]

logger = logging.getLogger("twine")


def get_config(path: str = "~/.pypirc") -> Dict[str, RepositoryConfig]:
# even if the config file does not exist, set up the parser
Expand Down Expand Up @@ -201,9 +204,9 @@ def check_status_code(response: requests.Response, verbose: bool) -> None:
except requests.HTTPError as err:
if response.text:
if verbose:
print("Content received from server:\n{}".format(response.text))
logger.info("Content received from server:\n{}".format(response.text))
else:
print("NOTE: Try --verbose to see response content.")
logger.warning("NOTE: Try --verbose to see response content.")
bhrutledge marked this conversation as resolved.
Show resolved Hide resolved
raise err


Expand Down