-
Notifications
You must be signed in to change notification settings - Fork 105
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
CLI: read config file from env location (Issue #189) #190
Conversation
This commit updates the way the config file is loaded by the Canopy CLI. When the config file location is not specified through the `-c` parameter, we use the file location specified in the environment variable CANOPY_COPY_FILE.
src/canopy_cli/cli.py
Outdated
@@ -102,7 +102,9 @@ def _initialize_tokenizer(): | |||
|
|||
def _read_config_file(config_file: Optional[str]) -> Dict[str, Any]: | |||
if config_file is None: | |||
return {} | |||
config_file = os.getenv("CANOPY_CONFIG_FILE", None) |
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.
I believe click
has a built-in solution.
If you'll look at the click declaration of the --index-name
argument of canopy new
, you'd notice an envvar=
option that that automatically sets that argument to also be read from an en var.
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.
LGTM!
(Have manually fixed PR comments that wasn't fixed for a week)
This PR fixes the bug described in issue #189.
Problem
The issue occurs then the user runs a Canopy CLI command (such as
new
,upsert
) and does not specify the-c
parameter with a config file lcoation.The current behaviour is to default to an empty (default) configuration while the user might expect Canopy to read the config file location specified in the environment variable
CANOPY_CONFIG_FILE
.Solution
This change updates the way the config file is loaded by the Canopy CLI. When the config file location is not specified through the
-c
parameter, we use the file location specified in the environment variable CANOPY_COPY_FILE.Type of Change
Test Plan
Only manual testing was done for this change.