feat:add new secret resolver to modules#231
Conversation
There was a problem hiding this comment.
This method shouldn't exist anymore
There was a problem hiding this comment.
Maybe instead of module, could it be service_name and instance_name?
There was a problem hiding this comment.
Could be moved to resolve.
I'm just thinking this is not an EnvVarResolver generic, because it expects module and instance. Should we maybe have an SdkEnvVarResolver and an EnvVarResolver just with the variable name?
There was a problem hiding this comment.
I believe this configuration could be global or per module.
There was a problem hiding this comment.
Aren't we missing an BindingRootResolver indeed?
There was a problem hiding this comment.
| def get() -> Optional[SdkConfig]: |
No need to duplicate sdk_config in the name since you are already have a class.
There was a problem hiding this comment.
Maybe we will need to have a generic list of args or an interface each resolver implement for ResolverParameters.
|
|
||
|
|
||
| def _load_config_from_env(instance: str = "default") -> DataAnonymizationConfig: | ||
| def _load_secrets(instance: str = "default") -> DataAnonymizationConfig: |
There was a problem hiding this comment.
I believe this encapsulation is not needed. We could also have an central enum for module names if we need it, instead of magic strings. What do you think?
Description
This PR implements the secret resolver into the sdk modules.
Type of Change
Please check the relevant option:
How to Test
Checklist
Before submitting your PR, please review and check the following:
Breaking Changes
If this PR introduces breaking changes, please describe:
Additional Notes
Add any additional context, screenshots, or information that would help reviewers.