-
Notifications
You must be signed in to change notification settings - Fork 122
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
docker_container: fix env_file option #452
docker_container: fix env_file option #452
Conversation
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.
This changes as are LGTM, but I have one suggestion.
The docs say:
- env_file
Path to a file, present on the target, containing environment variables `FOO=BAR'.
If variable also present in `env', then the `env' value will override.
[Default: (null)]
type: path
Perhaps it's also worthwhile to ensure that the overriding works properly (e.g. add a test with env_file: "{{ remote_tmp_dir }}/env-file"
and env: {TEST3: other_val3}
and make sure the value is other_val
)?
Docs Build 📝Thank you for contribution!✨ This PR has been merged and your docs changes will be incorporated when they are next published. |
@gotmax23 thanks for reviewing this! |
SUMMARY
The
env_file
option was ignored because of a snafu handling options withnot_a_container_option=True
. Unfortunately theenv_file
tests weren't really testing what happens, so they were happily passing...Fixes #451.
ISSUE TYPE
COMPONENT NAME
docker_container