feat: KeyringSettingsSource class to load keyring variables

[lmmx]

Proposal

PR to fulfill the feature proposal:

• Support keyring encrypted credential storage #139

Details

• A keyring_backend is provided, and behaves similarly to the env_file: if the named backend is not found, nothing is loaded. If not specified, the default backend is used.

mypy linting

The CI workflow does not install the project dependencies so the linting step is failing.

If I pip install -e[keyring] the mypy error is resolved.

Due to incorrectly cast annotations in the source package I had to use cast to get the correct annotation. Not all backends seem to support the enumeration of all keys in the keyring, so to begin with I have restricted the implementation to the SecretService backend (Linux).

pre-commit run --color never --all-files

check yaml...............................................................Passed
check toml...............................................................Passed
fix end of files.........................................................Passed
trim trailing whitespace.................................................Passed
Lint.....................................................................Passed
Mypy.....................................................................Passed
Pyupgrade................................................................Passed

Request for review

• Is the approach of try/except ImportError: pass acceptable? An alternative would be to set a variable that could be used to gate the calling of the keyring module.
• How should I implement error handling for the access of the variables? (As mentioned in the original proposal). If the keyring is locked it will raise an error. I haven't added this handling to keep the original proposal simple.

[hramezani]

Thanks @lmmx for the proposal and the PR 🙏

The CI workflow does not install the project dependencies so the linting step is failing.

You can add keyring to the linting requirements and then run pip-compile --output-file=requirements/linting.txt requirements/linting.in

Is the approach of try/except ImportError: pass acceptable? An alternative would be to set a variable that could be used to gate the calling of the keyring module.

We have email-validator as an optional dependency in Pydantic. Please take a look at pydantic.networks.py and especially import_email_validator.
Also, I think we need to include KeyringSettingsSource in sources if keyring is installed.

How should I implement error handling for the access of the variables? (As mentioned in the original proposal). If the keyring is locked it will raise an error. I haven't added this handling to keep the original proposal simple.

I think you can raise a SettingsError

I think we need to have:

• Proper support on Linux and Mac
• Tests
• Documentation

[lmmx]

TODO list

• pip compile
• Optional dependency import
• Settings source edit
• raise SettingsError upon keyring variable access error
• Linux support
  • Implemented via secretstorage as described
• Mac OSX support
• Tests
• Documentation

No comments 👍

• pip compile
• raise SettingsError upon keyring variable access error
• Linux support
• Documentation

Optional dependency import

The example of email_validator goes like so:

• TYPE_CHECKING conditional block that imports the optional dependency else sets it to None source

• Helper function called import_email_validator() that may raise ImportError, followed by differential declaration of a class that relies on it dependent on the TYPE_CHECKING context (if so, just declaring as Annotated[str, ...]). source

Settings source edit

Also, I think we need to include KeyringSettingsSource in sources if keyring is installed.

I hadn't thought of that, I just added it and let it default to None, I'll review...

Mac support

OS X has a security module which keyring can access and a dump-keychain method which would provide equivalent functionality. I need to look at this in more detail.

Tests

I used this as a dependency in an early stage project magic-scrape here (dependency declared here) and by patching the pydantic_settings.KeyringSettingsSource.__call__ method here (equivalently I could also have patched the _read_keyring method) I have been able to test the integration of this with my package.

This already helped me uncover and fix bugs in the implementation (in case sensitivity handling), so testing should be easy to build off of this.

[lmmx]

I thought about the way pydantic is importing optional dependencies and as I recalled seeing a cleaner way in pandas, I reviewed their approach, import_optional_dependency. source -> example usage

The rest of the comment explains why I'm choosing a combination of both, if it's of interest!

Theirs is not type equivalent: in pydantic the TYPE_CHECKING block ensures the type of the module name is always an imported module's type, whereas in pandas the return type of the helper function is Optional. This is only permissible there because they use --skip-unannotated and said helper function has no annotated return type.

Theirs is however cleaner: the import site is not required to be wrapped in if TYPE_CHECKING, and indeed their _optional.py module has scaled well to handle many optional deps.

pydantic's approach uses global to redefine the module which was imported as None in the else condition of the if TYPE_CHECKING block used to spoof the module type as if it were a non-Optional dependency.

We can parameterise globals()[package_name] instead and put the TYPE_CHECKING condition within the helper function, thereby achieving the same outcome (spoofed always-module that's actually Optional) but without the overhead at site of use. I expect this would look cleaner and hope this would therefore be acceptable in my PR.

In this case I would need to add multiple import lines to the block:

if TYPE_CHECKING:
    import keyring
    from keyring.backends.SecretService import Keyring as SecretServiceKeyring
else:
    keyring = None
    SecretServiceKeyring = None

as opposed to one line, without a TYPE_CHECKING block

keyring, SecretServiceKeyring = import_optional_dependency("keyring", "keyring.backends.SecretService.Keyring")

I'll put it together and fall back to the template of the single-module import function if I've misjudged part of this or if you don't like the idea.

[cisaacstern]

May I ask what the status of this PR is?