pypa
Add cli flag --force-keyring
Fixes #11020
Currently, Pip is conservative and does not query keyring at all when --no-input is used because the keyring might require user interaction such as prompting the user on the console.
These commits add a flag to pinky swear that the configured keyring backend does not require any user interaction. It defaults to False, making this opt-in behaviour.
Tools such as Pipx and Pipenv use Pip and have their own progress indicator that hides output from the subprocess Pip runs in. They should pass --no-input in my opinion, but Pip should provide some mechanism to still query the keyring in that case. Just not by default. So here we are.
If you supply the flag, but the keyring backend does prompt for user input (somehow, maybe via a GUI), what happens? I can imagine people using this flag getting very confused when the pip process "hangs" and reporting issues. In my experience, authentication prompts are notorious for "mostly" not needing user interaction, but then unexpectedly popping up prompts. The git client definitely does this - upgrading git often causes re-prompts, for example. And a keyring that periodically prompts for security reasons seems perfectly plausible to me. So it would be easy for a user to think that it's OK to supply this flag, but be wrong.
Also, the paragraph in the documentation about pipx/pipenv isn't at all clear to me. It seems to suggest that if you use those tools, you should always set this flag - which frankly seems like the wrong message to be giving.
Maybe this would be better handled as a --force-keyring flag, which overrides pip's current behaviour of disabling the keyring on --no-input. That's pretty easy to document ("Always query the keyring, regardless of pip's --no-input option. Note that this may cause problems if the keyring expects to be able to prompt the user interactively and no interactive user is available.") and puts the responsibility completely on the user to ensure that their keyring backend works as expected.
That does sound like a more sensible name and way to document the feature.
Time for a break tohopefully quicky stop being disapointed in myself not for realizing this myself after @uranusjr suggested --force-system-keyring. It was staring me right in the face and I wan't able to see it because I concentrated too much on system... Sorry!
I renamed the flag to --force-keyring and hopefully improved the documentation changes.
~~The flag name isn’t changed in documentation.~~ Edit: Need to change the config value and environment variable name as well.
~The flag name isn’t changed in documentation.~ Edit: Need to change the config value and environment variable name as well.
Woops. Fixed it.
leaving it for others to comment on.
@uranusjr It has been two months without any sign of Pip contributors doing so without any asking them to. Would pinging people or a team be reasonable at this point? (The latter is not something I can do as far as I can tell.)
I'm much the same as @uranusjr - the implementation looks OK, but I don't have much opinion on the design. Basically, I don't mind if someone else merges this, but I won't do so myself.
@judahrand I suspect this is of interest to you and your co-workers. It is less noticable, but if you are (indirect) users of virtualenv then you are affected by this last time I checked.
I don't have the bandwidth to review/decide what to do on this PR and, TBH, I'm not too sure about breaking behaviour change here without a migration period (x-ref https://pip.pypa.io/en/stable/development/release-process/#deprecation-policy). What do we want to do here folks?
If I do as @uranusjr suggests in https://github.com/pypa/pip/pull/11698 then this PR will become irrelevant and only --keyring-provider with the default of auto (current behaviour) should not require a deprecation period as far as I can tell.
I can have that refactor done in the next couple of days. Not that that will magically give you more bandwith to deal with that PR... Could you at least take a quick look at https://github.com/pypa/pip/issues/11658 which is about a tiny bug in new keyring related functionality on HEAD?
only --keyring-provider with the default of auto (current behaviour) should not require a deprecation period as far as I can tell.
Oh, this sounds great and I agree. :)
Could you at least take a quick look at #11658 which is about a tiny bug in new keyring related functionality on HEAD?
Have added it to the 23.0 milestone for now. That's a relatively easy fix, I think but if we can cover that as part of this PR, that works for me too. :)
only --keyring-provider with the default of auto (current behaviour) should not require a deprecation period as far as I can tell.
Oh, this sounds great and I agree. :)
I'm done with the refactoring.
I'm going to selfishly leave this PR open for now since this one is on de 23.0 milestone and the other is not so it serves as a reminder.
Given the lack of a response from the other maintainers here, and my lack of familiarity with this chunk of logic; I've extracted #11759 to fix the issue blocking the upcoming release based on the fix by @Darsstar related to that here.