Apache Airflow version
2.7.3
What happened
DockerOperator's env_file param is documented to accept a path:
:param env_file: Relative path to the ``.env`` file with environment variables to set in the container.
Overridden by variables in the environment parameter. (templated)
However, unpack_environment_variables() is implemented to load the str param as file_contents and not a path. Bug seems to have existed since the feature's introduction where the usage is:
operator = DockerOperator(
# ...
env_file="UNIT=FILE\nPRIVATE=FILE\nVAR=VALUE",
What you think should happen instead
Should load the path '.env' or other passed in path instead. Perhaps the use of StringIO was a leftover artifact of testing?
Although there's the option to consider the implemented functionality as "correct" and deem this a documentation bug, a "pass in file contents as text" interface is not desirable compared to the behavior as documented, esp since it's preferable to match the interface of docker run, which states:
--env-file Read in a file of environment variables
Implementation Notes:
- The
unpack_environment_variables() function is wholly unnecessary, providing little to no abstraction value. Should just inline the call to dotenv_values().
dotenv_values() is nicely typed, accepting a dotenv.StrPath type as the first arg. DockerOperator's env_file param use the same annotated type (specifically, str | os.PathLike[str] | None, or dotenv.StrPath | None)
How to reproduce
Use DockerOperator as written in tests
Operating System
n/a
Versions of Apache Airflow Providers
name = "apache-airflow-providers-docker"
version = "3.8.1"
Deployment
Docker-Compose
Deployment details
n/a
Anything else
What's Airflow's SOP on minor breaking changes? Is it necessary to have a whole deprecation cycle of warnings etc for this?
Are you willing to submit PR?
Code of Conduct
Apache Airflow version
2.7.3
What happened
DockerOperator'senv_fileparam is documented to accept a path:However,
unpack_environment_variables()is implemented to load thestrparam as file_contents and not a path. Bug seems to have existed since the feature's introduction where the usage is:What you think should happen instead
Should load the path
'.env'or other passed in path instead. Perhaps the use ofStringIOwas a leftover artifact of testing?Although there's the option to consider the implemented functionality as "correct" and deem this a documentation bug, a "pass in file contents as text" interface is not desirable compared to the behavior as documented, esp since it's preferable to match the interface of
docker run, which states:Implementation Notes:
unpack_environment_variables()function is wholly unnecessary, providing little to no abstraction value. Should just inline the call todotenv_values().dotenv_values()is nicely typed, accepting adotenv.StrPathtype as the first arg.DockerOperator'senv_fileparam use the same annotated type (specifically,str | os.PathLike[str] | None, ordotenv.StrPath | None)How to reproduce
Use
DockerOperatoras written in testsOperating System
n/a
Versions of Apache Airflow Providers
Deployment
Docker-Compose
Deployment details
n/a
Anything else
What's Airflow's SOP on minor breaking changes? Is it necessary to have a whole deprecation cycle of
warningsetc for this?Are you willing to submit PR?
Code of Conduct