-
Notifications
You must be signed in to change notification settings - Fork 4
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
Allow configuration to be set for session type #70
Conversation
This will allow folks using custom cred types (like LastPass!) to configure this globally via config files.
@@ -96,7 +96,7 @@ URL or configure a TOML-style file in one of the following locations: | |||
|
|||
Example file: | |||
|
|||
```toml | |||
```ini |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Turns out config_resolver uses https://docs.python.org/3/library/configparser.html which uses a simplified INI format which is not actually TOML. TIL!
@@ -109,3 +109,72 @@ downloaded for local processing) will be stored per Python's | |||
which allow for configuration via the `TMPDIR`, `TEMP` or `TMP` env | |||
variables, and generally default to | |||
[something reasonable per your OS](https://docs.python.org/3/library/tempfile.html#tempfile.gettempdir). | |||
|
|||
## Cloud credentials (e.g., S3/GCS/Google Sheets) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since database credentials come out of db-facts, the major difference made in switching the default creds provider from CredsViaLastPass and CredsViaEnv is how non-default GCP credentials are passed--this text should address that and help provide guidance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense-- but wondering whether "session type" is the right terminology here for controlling where creds come from.
cfg = config_result.config | ||
if 'session' in cfg: | ||
session_cfg = cfg['session'] | ||
session_type: Optional[str] = session_cfg.get('session_type') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could creds_type
be a more appropriate name here? Does this control anything about the session other than how it gets the creds?
This will allow folks using custom cred types (like LastPass!) to configure this globally via config files. Combined with #69 this will allow enterprise uses to be configured while more generic settings are used by default.