Drop plan environment from the default command environment#4613
Drop plan environment from the default command environment#4613
Conversation
When guests execute commands, plan environment is passed to all commands. This spoils the order of precedence. Patch removes this environment source, which is only used when `Guest.execute()` did not provide any environment of its own.
happz
added
area | environment
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request updates the logic for preparing command environments to respect precedence. The review suggests simplifying environment creation in tmt/guest/__init__.py for better readability. Additionally, the GuestLocal.execute method in tmt/steps/provision/local.py still needs updating for consistency.
| environment = tmt.utils.Environment() | ||
|
|
||
| environment.update( | ||
| self.environment, | ||
| self.parent.plan.environment, | ||
| ) |
There was a problem hiding this comment.
Simplify environment creation for better readability. The custom update() method accepting multiple arguments is less conventional than using copy() and a subsequent update().
| environment = tmt.utils.Environment() | |
| environment.update( | |
| self.environment, | |
| self.parent.plan.environment, | |
| ) | |
| environment = self.environment.copy() | |
| environment.update(self.parent.plan.environment) |
LecrisUT
left a comment
LecrisUT
left a comment
There was a problem hiding this comment.
Not technically a "no functional change" since it changes how Guest.environment is injected (previously it was not?), but it should have been the case so 👍
| When the environment is unset, construct a basic environment | ||
| from the plan and run. Otherwise, the environment is left | ||
| unmodified. |
There was a problem hiding this comment.
Can you link this more directly to :param: environment? "When the environment is unset" is a bit too vague
2 participants
When guests execute commands, plan environment is passed to all commands. This spoils the order of precedence. Patch removes this environment source, which is only used when
Guest.execute()did not provide any environment of its own.Pull Request Checklist