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

Add envs parameter to compose up/config/run #380

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lugoues
Copy link

@lugoues lugoues commented Nov 6, 2022

Docker compose allows variable injection from the process environment in addition to a .env file. This change will allow user the ability to pass a dictionary of variables to be injected into the compose subprocess.

@lugoues lugoues force-pushed the compose_env_vars branch 2 times, most recently from 6c49ed4 to e3dd810 Compare November 6, 2022 22:22
@gabrieldemarmiesse
Copy link
Owner

Thanks for the pull request, that parameter should be added to the client constructor, similarely to other compose parameters like the file path or project name. Thanks!

@lugoues
Copy link
Author

lugoues commented Nov 6, 2022

Thanks for the pull request, that parameter should be added to the client constructor, similarely to other compose parameters like the file path or project name. Thanks!

I had avoided adding it to the client as it is not a flag and all the supporting code only deals with flags. The method I would take to implement this would be:

  1. add compose_envs to DockerClient constructor
  2. add compose_envs to ClientConfig
  3. add envs to all run statements, pulling from self.client_config.compose_envs within the ComposeCLI class.

This begs the question, would we want to support the envs for every subprocess call in the client? The container class has several calls where it accepts envs that we could set at the client level.

@gabrieldemarmiesse
Copy link
Owner

Similar to how we use "self.docker_compose_cmd" in every call to compose, we should use "self.docker_compose_envs". Because compose commands parse the compose file to understand what is available and how to behave with services, we should passe it to all commands. For a better user experience, we wouldn't want the user to specify those environement variables every time. Just once in the DockerClient constructor seems fine :) Similar to all the other compose flags

@lugoues lugoues force-pushed the compose_env_vars branch 2 times, most recently from f71c924 to 68e1e7f Compare November 19, 2022 05:43
@lugoues lugoues changed the title Add envs parameter to compose up/config Add envs parameter to compose up/config/run Nov 19, 2022
@lugoues lugoues marked this pull request as draft November 19, 2022 05:56
@timini
Copy link

timini commented Dec 19, 2022

any news on this?

@lugoues
Copy link
Author

lugoues commented Dec 20, 2022 via email

@Tardo
Copy link

Tardo commented Aug 23, 2023

Thanks for the work... any news?

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.

4 participants