potiuk commented on a change in pull request #21145:
URL: https://github.com/apache/airflow/pull/21145#discussion_r808901770
##########
File path: dev/breeze/src/airflow_breeze/shell/enter_shell.py
##########
@@ -117,18 +116,15 @@
}
-def construct_arguments_docker_compose_command(shell_params: ShellBuilder) ->
List[str]:
- args_command = []
+def construct_env_variables_docker_compose_command(shell_params: ShellBuilder)
-> Dict[str, str]:
+ env_variables: Dict[str, str] = {}
for param_name in PARAMS_TO_ENTER_SHELL:
param_value = PARAMS_TO_ENTER_SHELL[param_name]
- os.environ[param_name] = str(getattr(shell_params, param_value))
- args_command.append(param_name + '=' + str(getattr(shell_params,
param_value)))
+ env_variables[param_name] = str(getattr(shell_params, param_value))
Review comment:
Here. Instead of modifying the "os.environ" I simply store the values
in the dictionary and return iit.
Updating os.environ here is not really good because:
1) It modifies the env variables for whole process, not only for the command
we want to run
2) It is a bit "unexpected side effect" of running the function. Generally
functions like that should not modify any "global" state - they should at most
take input and produce output (This is "functional" programming pattern).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
