Add use_env_var flag to client#923
Conversation
Signed-off-by: Jun Ki Min <[email protected]>
Signed-off-by: Jun Ki Min <[email protected]>
Signed-off-by: Jun Ki Min <[email protected]>
Signed-off-by: Jun Ki Min <[email protected]>
Signed-off-by: Jun Ki Min <[email protected]>
Signed-off-by: Jun Ki Min <[email protected]>
Signed-off-by: Jun Ki Min <[email protected]>
Yuqing-cat
left a comment
There was a problem hiding this comment.
The enhancement looks reasonable. It will be better if we could make _EnvVariableUtil less complex.
|
Hello @loomlike seems this doc should get updated as well https://feathr-ai.github.io/feathr/how-to-guides/feathr-configuration-and-env.html#configuration-and-environment-variables-in-feathr Can I ask is there any use case for introducing this new env? Have this new flag can make configuration more flexible but it introduces complexity on configuration code as well. |
The default behavior is to override the config file values ( |
Signed-off-by: Jun Ki Min <[email protected]>
Yuqing-cat
left a comment
There was a problem hiding this comment.
Looks nice to me! Thanks for the changes.
Please also change the file names envvariableutil.py & test_envvariableutil.py to fit the new class name.
Signed-off-by: Jun Ki Min <[email protected]>
Signed-off-by: Jun Ki Min <[email protected]>
jainr
left a comment
There was a problem hiding this comment.
Thanks for making the changes and addressing the comments.
Signed-off-by: Jun Ki Min [email protected]
Description
Introduce an option to select between env vars and config yaml file.
Feathr client to use explicitly configured yaml file over environment variable if
use_env_varflag is set to False.The changes are added as the last argument of the existing functions and set default to True (use env variables) so that
existing codes don't break.
Resolves #922
How was this PR tested?
Unit test
Does this PR introduce any user-facing changes?