From 9058b900214579a8430e5aaec383d61c9608022a Mon Sep 17 00:00:00 2001 From: Frost Ming Date: Fri, 3 Feb 2023 09:32:53 +0800 Subject: [PATCH 1/3] fix: correct the way to decide if keyring is available --- src/pip/_internal/network/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pip/_internal/network/auth.py b/src/pip/_internal/network/auth.py index c1621326826..a1c52315b2b 100644 --- a/src/pip/_internal/network/auth.py +++ b/src/pip/_internal/network/auth.py @@ -359,7 +359,7 @@ def _prompt_for_password( # Factored out to allow for easy patching in tests def _should_save_password_to_keyring(self) -> bool: - if get_keyring_provider() is None: + if isinstance(get_keyring_provider(), KeyRingNullProvider): return False return ask("Save credentials to keyring [y/N]: ", ["y", "n"]) == "y" From 706456c5cf463f3ed8f1a949a71bf4379e6baf3a Mon Sep 17 00:00:00 2001 From: Frost Ming Date: Fri, 3 Feb 2023 09:39:21 +0800 Subject: [PATCH 2/3] add news --- news/11774.bugfix.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/11774.bugfix.rst diff --git a/news/11774.bugfix.rst b/news/11774.bugfix.rst new file mode 100644 index 00000000000..771246b0b54 --- /dev/null +++ b/news/11774.bugfix.rst @@ -0,0 +1 @@ +Correct the way to decide if keyring is available. From 2d0a5c9cd29f72348031b8b517068f98aed14ad7 Mon Sep 17 00:00:00 2001 From: Frost Ming Date: Fri, 3 Feb 2023 15:33:55 +0800 Subject: [PATCH 3/3] use a attribute to tell if the provider is null --- src/pip/_internal/network/auth.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/pip/_internal/network/auth.py b/src/pip/_internal/network/auth.py index a1c52315b2b..ac8cbf23bab 100644 --- a/src/pip/_internal/network/auth.py +++ b/src/pip/_internal/network/auth.py @@ -39,6 +39,8 @@ class Credentials(NamedTuple): class KeyRingBaseProvider(ABC): """Keyring base provider interface""" + has_keyring: bool + @abstractmethod def get_auth_info(self, url: str, username: Optional[str]) -> Optional[AuthInfo]: ... @@ -51,6 +53,8 @@ def save_auth_info(self, url: str, username: str, password: str) -> None: class KeyRingNullProvider(KeyRingBaseProvider): """Keyring null provider""" + has_keyring = False + def get_auth_info(self, url: str, username: Optional[str]) -> Optional[AuthInfo]: return None @@ -61,6 +65,8 @@ def save_auth_info(self, url: str, username: str, password: str) -> None: class KeyRingPythonProvider(KeyRingBaseProvider): """Keyring interface which uses locally imported `keyring`""" + has_keyring = True + def __init__(self) -> None: import keyring @@ -97,6 +103,8 @@ class KeyRingCliProvider(KeyRingBaseProvider): PATH. """ + has_keyring = True + def __init__(self, cmd: str) -> None: self.keyring = cmd @@ -359,7 +367,7 @@ def _prompt_for_password( # Factored out to allow for easy patching in tests def _should_save_password_to_keyring(self) -> bool: - if isinstance(get_keyring_provider(), KeyRingNullProvider): + if not get_keyring_provider().has_keyring: return False return ask("Save credentials to keyring [y/N]: ", ["y", "n"]) == "y" @@ -432,9 +440,7 @@ def warn_on_401(self, resp: Response, **kwargs: Any) -> None: def save_credentials(self, resp: Response, **kwargs: Any) -> None: """Response callback to save credentials on success.""" keyring = get_keyring_provider() - assert not isinstance( - keyring, KeyRingNullProvider - ), "should never reach here without keyring" + assert keyring.has_keyring, "should never reach here without keyring" creds = self._credentials_to_save self._credentials_to_save = None