Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should third party keyring backends use keyringrc.cfg, or use their own config file? #682

Open
jfly opened this issue Jun 4, 2024 · 2 comments · May be fixed by #702
Open

Should third party keyring backends use keyringrc.cfg, or use their own config file? #682

jfly opened this issue Jun 4, 2024 · 2 comments · May be fixed by #702

Comments

@jfly
Copy link

jfly commented Jun 4, 2024

I'm using the keyrings.codeartifact backend, which expects its configuration to be stored in keyring's keyringrc.cfg file:

Is this supported? Or should keyrings.codeartifact look for its own config file? Reusing the config file feels a little odd to me, as there's potential for naming conflicts, but maybe that's unlikely if folks are reasonable about naming.

If this is supported, then I think keyring is overly strict in how it parses its config file:

I can work around this by explicitly setting the backend.default-keyring setting, but I'd rather not set a configuration option that I don't have to just to avoid a warning.

@jaraco
Copy link
Owner

jaraco commented Jun 30, 2024

To my recollection, it hasn't been explicitly supported or not supported. I suspect other backends like keyrings.alt re-use the same platform module for configuration, so I'm not surprised that other backends are using it. I'm inclined to say it should be supported.

And I agree with your assessment - adding unrelated config shouldn't break the behavior compared to an empty config.

Are you willing/interested to work on a fix?

jfly added a commit to jfly/keyring that referenced this issue Nov 10, 2024
This config file is used by third party keyring backends, and setting
one of *their* settings shouldn't give you a warning encouraging you to set `backend`
setting.

This fixes jaraco#682
@jfly
Copy link
Author

jfly commented Nov 10, 2024

Are you willing/interested to work on a fix?

Sure! How does this look? #702

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants