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

Improve logging when keyring fails #889

Closed
fedexist opened this issue Mar 31, 2022 · 4 comments · Fixed by #890
Closed

Improve logging when keyring fails #889

fedexist opened this issue Mar 31, 2022 · 4 comments · Fixed by #890

Comments

@fedexist
Copy link
Contributor

Your Environment

  1. Your operating system: docker container with the following /etc/os-release
NAME="Ubuntu"
VERSION="18.04.5 LTS (Bionic Beaver)"
ID=ubuntu
ID_LIKE=debian
PRETTY_NAME="Ubuntu 18.04.5 LTS"
VERSION_ID="18.04"
HOME_URL="https://www.ubuntu.com/"
SUPPORT_URL="https://help.ubuntu.com/"
BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/"
PRIVACY_POLICY_URL="https://www.ubuntu.com/legal/terms-and-policies/privacy-policy"
VERSION_CODENAME=bionic
UBUNTU_CODENAME=bionic
  1. Version of python you are running: python3.9

  2. How did you install twine? Did you use your operating system's package manager or pip or something else? Running in a virtual environment created by tox

  3. Version of twine you have installed (include complete output of): 4.0.0

  4. Which package repository are you targeting? Artifact Registry on Google Cloud Platform, using the additional keyring: keyrings.google-artifactregistry-auth.

The Issue

When launching twine from within a tox step, defined as follows:

[testenv:publish]
description =
    Publish the package you have been developing to a package index server.
    By default, it uses testpypi. If you really want to publish your package
    to be publicly accessible in PyPI, use the `-- --repository pypi` option.
skip_install = True
changedir = {toxinidir}
basepython = python3.9
passenv =
    PYPI_REPO
    GOOGLE_APPLICATION_CREDENTIALS
deps =
    twine
commands =
    python3.9 -m twine check dist/*
    python3.9 -m twine upload --skip-existing --verbose --non-interactive {posargs:--repository-url {env:PYPI_REPO}} dist/*

when uploading there was a warning saying

Uploading distributions to 
https://europe-west3-python.pkg.dev/project_id/pypi/
�[34mINFO    �[0m dist/package-0.1.dev55+g42d0d37-py2.py3-none-any.whl (17.0 KB)       
�[34mINFO    �[0m dist/package-0.1.dev55+g42d0d37.tar.gz (28.8 KB)                     
�[34mINFO    �[0m Querying keyring for username                                          
�[33mWARNING �[0m 'getpwuid(): uid not found: 999'                                       
ERROR: InvocationError for command /.tox/publish/bin/python3.9 -m twine upload --skip-existing --verbose --non-interactive --repository-url https://europe-west3-python.pkg.dev/project_id/pypi/ 'dist/*' (exited with code 1)

Iand then failing to upload the package.

After investigating, I've found out that this is due to keyring failing with the exception

  File "/usr/lib/python3.9/pathlib.py", line 376, in gethomedir
    return os.environ['HOME']
  File "/usr/lib/python3.9/os.py", line 679, in __getitem__
    raise KeyError(key) from None
KeyError: 'HOME'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File ".tox/publish/lib/python3.9/site-packages/keyring/core.py", line 72, in get_credential
    return get_keyring().get_credential(service_name, username)
  File ".tox/publish/lib/python3.9/site-packages/keyring/core.py", line 32, in get_keyring
    init_backend()
  File ".tox/publish/lib/python3.9/site-packages/keyring/core.py", line 83, in init_backend
    set_keyring(_detect_backend(limit))
  File ".tox/publish/lib/python3.9/site-packages/keyring/core.py", line 98, in _detect_backend
    or load_config()
  File ".tox/publish/lib/python3.9/site-packages/keyring/core.py", line 153, in load_config
    keyring_cfg = os.path.join(platform.config_root(), filename)
  File ".tox/publish/lib/python3.9/site-packages/keyring/util/platform_.py", line 59, in _config_root_Linux
    _check_old_config_root()
  File ".tox/publish/lib/python3.9/site-packages/keyring/util/platform_.py", line 43, in _check_old_config_root
    config_file_new = os.path.join(_config_root_Linux(), 'keyringrc.cfg')
  File ".tox/publish/lib/python3.9/site-packages/keyring/util/platform_.py", line 60, in _config_root_Linux
    fallback = pathlib.Path.home() / '.config'
  File "/usr/lib/python3.9/pathlib.py", line 1132, in home
    return cls(cls()._flavour.gethomedir(None))
  File "/usr/lib/python3.9/pathlib.py", line 379, in gethomedir
    return pwd.getpwuid(os.getuid()).pw_dir
KeyError: 'getpwuid(): uid not found: 999'

Of course, after seeing this error, the issue was clear and I then added the HOME environment variable to passenv in tox.ini.

passenv =
    PYPI_REPO
    GOOGLE_APPLICATION_CREDENTIALS
    HOME

Steps to Reproduce

Just unset HOME before running twine.

It would be nice if the warning included the traceback from the keyring exception, which is basically silenced by the current implementation in auth.py:

    def get_username_from_keyring(self) -> Optional[str]:
        try:
            system = cast(str, self.system)
            logger.info("Querying keyring for username")
            creds = keyring.get_credential(system, None)
            if creds:
                return cast(str, creds.username)
        except AttributeError:
            # To support keyring prior to 15.2
            pass
        except Exception as exc:
            logger.warning(str(exc))
        return None

maybe using exc_info

logger.warning(str(exc), exc_info=exc)

?

@sigmavirus24
Copy link
Member

We should just use logger.exception and let that get the appropriate details

@fedexist
Copy link
Contributor Author

fedexist commented Apr 4, 2022

We should just use logger.exception and let that get the appropriate details

I wasn't familiar with logger.exception, but that looks good enough in this case, if we're ok with having an ERROR logged, rather than a WARNING, but since we're talking about exception handling, I think that would be enough.

Can I go ahead and make a PR for this?

@bhrutledge
Copy link
Contributor

I'm not entirely sure what a complete solution is yet, but starting with a PR sounds good. I'm happy to work through it from there.

@fedexist When you open the PR, it'd be helpful to have screenshots of the output.

@sigmavirus24
Copy link
Member

Also please paste the exact text for those who can't see or rely on screenshots

@bhrutledge bhrutledge linked a pull request May 21, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants