-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Use environment variable per run, not per container #6480
Use environment variable per run, not per container #6480
Conversation
Currently we pass the environment vars in the init method and create the container with that. This means that we can't modify those after the container creation. We do allow this in the LocalBuildEnvironment. This is needed to run git in a container (set envs like `GIT_DIR`). And to execute the ssh agent inside, (sent envs like `SSH_AUTH_SOCKET` and `SSH_AGENT_PID`)
I'm not sure to understand what are the benefits of this PR and why we need this. Can you explain a little more here?
Can't these variables set at the moment the container is created instead? |
But we initialize the ssh agent (set the env vars) after the container is created. |
We need to do: Create container -> Run |
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 makes a lot of sense 👍
@@ -452,7 +453,7 @@ def run_command_class( | |||
kwargs['bin_path'] = env_path | |||
if 'environment' in kwargs: | |||
raise BuildEnvironmentError('environment can\'t be passed in via commands.') | |||
kwargs['environment'] = self.environment | |||
kwargs['environment'] = self.environment.copy() |
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.
Are we always calling copy
so we don't accidentally modify it in memory?
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.
Yes
Currently we pass the environment vars in the init method
and create the container with that.
This means that we can't modify those after the container creation.
We do allow this in the LocalBuildEnvironment.
This is needed to run git in a container (set envs like
GIT_DIR
).And to execute the ssh agent inside, (sent envs like
SSH_AUTH_SOCKET
and
SSH_AGENT_PID
)This is compatible with current code, so it can be merged just now.