-
Notifications
You must be signed in to change notification settings - Fork 308
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
Credential Logging #685
Credential Logging #685
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good stuff! Nice job wading through the complexity of how the username and credentials are set. My requests are just to normalize the log messages.
For example:
$ twine upload --verbose -r testpypi --disable-progress-bar dist/*
Using configuration from /Users/brian/.pypirc
Uploading distributions to https://test.pypi.org/legacy/
dist/example_pkg_bhrutledge-0.0.4a6-py3-none-any.whl (2.6 KB)
dist/example-pkg-bhrutledge-0.0.4a6.tar.gz (1.3 KB)
username set from config file
password set from keyring
username: __token__
password: <hidden>
Uploading example_pkg_bhrutledge-0.0.4a6-py3-none-any.whl
Uploading example-pkg-bhrutledge-0.0.4a6.tar.gz
$ twine upload --verbose -r testpypi -u bhrutledge --disable-progress-bar dist/*
Using configuration from /Users/brian/.pypirc
Uploading distributions to https://test.pypi.org/legacy/
dist/example_pkg_bhrutledge-0.0.4a6-py3-none-any.whl (2.6 KB)
dist/example-pkg-bhrutledge-0.0.4a6.tar.gz (1.3 KB)
username set by command options
Enter your password:
username: bhrutledge
password: <empty>
Uploading example_pkg_bhrutledge-0.0.4a6-py3-none-any.whl
...
HTTPError: 403 Forbidden from https://test.pypi.org/legacy/
Invalid or non-existent authentication information. See https://test.pypi.org/help/#invalid-auth for more information.
@VikramJayanthi17 FYI, I've merged a handful of changes for unrelated failing tests on |
@VikramJayanthi17 FYI, I've merged some formatting changes that will likely cause a merge conflict on this PR. |
@di I haven't heard from @VikramJayanthi17 in awhile. Is it okay if I commit my suggestions and finish up this PR? |
@bhrutledge Yes, his internship has ended and he's back at school now. I think it's probably fine for you to take over here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finally got around to adding some tests, which uncovered a small UX tweak. I'm very happy to merge this. @VikramJayanthi17 thanks again for all of your work!
This PR implements a basic version of credential logging as mentioned in the #381 checklist, . This is a draft PR as I was unsure of how we want to implement the specifics. This is how this functionality is described(by @bhrutledge) in the checklist:
The questions I have are:
Any feedback on this PR or answers to these questions would be greatly appreciated.
TODO: