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

Add instructions on disabling keyring #7200

Closed
wants to merge 9 commits into from
Closed

Conversation

Suniti13
Copy link

@Suniti13 Suniti13 commented Oct 14, 2019

Added documentation about how to disable keyring. Refered pypa/twine#338 for it.

Fixes #6773.

Suniti13 and others added 5 commits October 14, 2019 16:54
Added documentation about how to disable keyring. Refered pypa/twine#338 for it.
Small edit because of whitespaces
@chrahunt chrahunt changed the title Fix to issue #6773 Add instructions on disabling keyring Oct 14, 2019
@pradyunsg pradyunsg closed this Oct 15, 2019
@pradyunsg pradyunsg reopened this Oct 15, 2019
Copy link
Member

@pradyunsg pradyunsg left a comment

Choose a reason for hiding this comment

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

Hi! Thanks for the PR @Suniti13!

A few minor stylistic suggestions. Happy to merge once they're fixed! ^>^

docs/html/user_guide.rst Outdated Show resolved Hide resolved
docs/html/user_guide.rst Outdated Show resolved Hide resolved
docs/html/user_guide.rst Outdated Show resolved Hide resolved
docs/html/user_guide.rst Outdated Show resolved Hide resolved
docs/html/user_guide.rst Outdated Show resolved Hide resolved
@pradyunsg pradyunsg added the type: docs Documentation related label Oct 15, 2019
Added NEWS entry for the issue pypa#6773. Added documentation on how to disable keyring.
@pradyunsg pradyunsg closed this Oct 18, 2019
@pradyunsg pradyunsg reopened this Oct 18, 2019
Copy link
Member

@xavfernandez xavfernandez left a comment

Choose a reason for hiding this comment

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

It might be interesting to introduce keyring and its purpose with a sentence and maybe a link before explaining how to disable it ?

Keyring in certain cases can prevent the installing of certain packages and modules
due to authentication errors. In such cases disabling the keyring is recommended.
Keyring can be manually uninstalled but doing so may invalidate other packages that
depends on Keyring.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
depends on Keyring.
depend on Keyring.

Copy link
Member

@techalchemy techalchemy left a comment

Choose a reason for hiding this comment

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

Thanks for making this PR, I have been working through some keyring related issues today and I was wondering whether it is discussed at all in the documentation so it is good to see it being added! I am not a maintainer on this project but I made some suggestions anyway, feel free to consult with someone who can hit the merge button if you have any hesitations. Thanks again for your contribution!

Disable Keyring
***************

Keyring in certain cases can prevent the installing of certain packages and modules
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps we can indicate how a user could recognize if they are impacted by this issue? The language here is a bit vague, perhaps there are some example warnings (maybe they are ResourceWarnings or Exceptions that would indicate if keyring is causing this problem).

Copy link

Choose a reason for hiding this comment

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

By default you don't get any output unfortunately. If you increase the verbosity you are going to read something like Getting credentials from keyring for https://pypi.org/simple before it get stuck

Disable Keyring
***************

Keyring in certain cases can prevent the installing of certain packages and modules
Copy link
Member

Choose a reason for hiding this comment

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

Also I don't think keyring is mentioned anywhere else in the user guide, maybe before talking about how to turn it off, you could mention that pip doesn't come with keyring but it may try to use it if the user has it installed.

Keyring in certain cases can prevent the installing of certain packages and modules
due to authentication errors. In such cases disabling the keyring is recommended.
Keyring can be manually uninstalled but doing so may invalidate other packages that
depends on Keyring.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
depends on Keyring.
depends on keyring.

@brainwane
Copy link
Contributor

@Suniti13 Hi! It would be great if you could respond to the review comments people have left. Once you resolve those suggestions, the maintainers can merge your improvements into the master branch, and once the new documentation is deployed, users can enjoy your improvements!

@BrownTruck
Copy link
Contributor

Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the master branch into this pull request or rebase this pull request against master then it will be eligible for code review and hopefully merging!

@BrownTruck BrownTruck added the needs rebase or merge PR has conflicts with current master label Mar 29, 2020
@pradyunsg
Copy link
Member

Closing as per #8019. Thanks @Suniti13!

@pradyunsg pradyunsg closed this Apr 27, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs rebase or merge PR has conflicts with current master type: docs Documentation related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document how to disable keyring for users who might want to
7 participants