Skip to content
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

Fix environment variable precedents #771

Merged
merged 1 commit into from
Jun 22, 2024

Conversation

wgnathanael
Copy link
Contributor

Allow environment variables set in the host os to override the value of a variable defined in a .env file.
This is consistent with docker's environment variable precedence. Per https://docs.docker.com/compose/environment-variables/envvars-precedence/#advanced-example

Copy link
Collaborator

@p12tic p12tic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need proper test for this.

@wgnathanael wgnathanael force-pushed the environment-precedence branch from eea42d7 to 298e6d1 Compare March 21, 2024 14:47
@wgnathanael
Copy link
Contributor Author

I'd love to submit some tests but I can't even find existing tests that test the env/env-file features as is. Is there one I can use a template someplace?

@charliemirabile
Copy link
Contributor

@p12tic bump on this issue. I (and presumably @wgnathanael) would love to get this merged as I am currently relying on it in my own fork to be able to use podman-compose in production, but it needs tests. Happy to contribute some, but as @wgnathanael says, there doesn't seem to be any infrastructure in place for testing the existing environment variable code. Do you have any sense of how this could be tested in a reasonably straightforward way?

@p12tic
Copy link
Collaborator

p12tic commented Jun 22, 2024

Thanks for the bump, I somehow missed @wgnathanael comment. If there is no testing infrastructure for a feature, it's more of a responsibility of the maintainer to define one instead of infrequent contributors. I will merge this PR as is.

@p12tic p12tic force-pushed the environment-precedence branch from 298e6d1 to de05098 Compare June 22, 2024 16:55
@p12tic
Copy link
Collaborator

p12tic commented Jun 22, 2024

Rebased and fixed merge conflict.

@p12tic p12tic force-pushed the environment-precedence branch from de05098 to 935029d Compare June 22, 2024 16:58
@p12tic p12tic merged commit e07c28d into containers:main Jun 22, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants