feature: Allow listing of acct #665
Conversation
|
Hi @mitya57 , |
Note: I'm not the chief maintainer of Keyring, @jaraco is, so he has the final word. |
There was a problem hiding this comment.
Overall, this looks good. I have a couple of comments about naming and the design.
I'm also curious - what should this project think about the unimplemented backends? Ideally, a feature like this would be launched with support in Mac, Windows, and Linux. Would you be willing to explore an implementation that works in those backends?
Finally, this change should demand some tests. Let's make sure the test suite exercises this new expectation and functionality.
| @@ -100,6 +100,11 @@ def get_password(self, service: str, username: str) -> typing.Optional[str]: | |||
| """Get password of the username for the service""" | |||
| return None | |||
|
|
|||
| @abc.abstractmethod | |||
| def list_password(self, service: str) -> list[str]: | |||
There was a problem hiding this comment.
How about name it list_accounts or list_users, since "username" and "account" are the terms used for these fields and "passwords" aren't involved in the result.
| @@ -87,6 +87,8 @@ or with ``--help`` as so:: | |||
| Password for 'username' in 'system': | |||
| $ keyring get system username | |||
| password | |||
| $ keyring list app_name | |||
There was a problem hiding this comment.
Probably this should mention "system" and not "app_name", because "system" is what's mentioned above.
| @@ -87,6 +87,8 @@ or with ``--help`` as so:: | |||
| Password for 'username' in 'system': | |||
| $ keyring get system username | |||
| password | |||
| $ keyring list app_name | |||
There was a problem hiding this comment.
We should think carefully about the "command" here. It's called "list", but "list" could mean "list users" or "list systems" or "list users and systems". I wonder if we shouldn't instead allow a generic "list" that takes a string and matches any system:password where that string is found. Let's take some time to think about other use-cases other than exact matches of a system.
|
This PR appears to be languishing, so I'm going to close it for now. Feel free to revive the conversation and respond to the questions and we can re-open. |
This PR add usage of listing accounts given an app name.
Then you will be able to access an specific account using: