Store/retrieve passwords with the system keyring #1499
Conversation
| if show_warnings: | ||
| mycli.show_warnings = show_warnings | ||
|
|
||
| if use_keyring_cli_opt and use_keyring_cli_opt.lower() == 'reset': |
There was a problem hiding this comment.
You're pulling in True/False values as strings, so all of the logic needs to account for that. Easiest probably would be to just check the option variable and then set a new real bool var to use for your logic.
There was a problem hiding this comment.
Isn't that exactly what we are doing, where the "new real bool var" is use_keyring?
There was a problem hiding this comment.
It is; I was mixing it up with the other suggestions because use_keyring_cli_opt is still a string and in this IF it is being treated like a bool. It works because all that really matters is that use_keyring_cli_opt is not none so you can .lower it. So I would recommend switching it to be more clear
if use_keyring_cli_opt is not None and use_keyring_cli_opt.lower() == 'reset':
That (to me at least) makes it clear what you're doing and that you don't actually care that it's a bool (or not really a bool in this case)
mycli/main.py
Outdated
| use_keyring = True | ||
| reset_keyring = True | ||
| elif use_keyring_cli_opt is None: | ||
| use_keyring = mycli.config['main'].get('use_keyring') |
There was a problem hiding this comment.
Similar to above, will need as_bool to treat it as such.
There was a problem hiding this comment.
Right! Great catch. Our config library should really do that for us.
* off by default * can be enabled by setting use_keyring = True in ~/.myclirc * can be enabled on a per-connection basis with --use-keyring=true at invocation time * note that the hostname is considered to be different if short or qualified * a password can be reset with --use-keyring=reset at invocation time, or (not documented) using the "keyring" CLI tool
rolandwalker
force-pushed
the
RW/save-passwords-in-system-keyring
branch
from
0d21049 to
2276391
Compare
Description
use_keyring = Truein~/.myclirc.--use-keyring=trueat invocation time.--use-keyring=resetat invocation time, or (not documented) using thekeyringCLI tool.Fixes #1496.
This was super easy to implement! But realistic tests look quite difficult.
Checklist
changelog.md.AUTHORSfile (or it's already there).uv run ruff check && uv run ruff format && uv run mypy --install-types .to lint and format the code.