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

Remove Channel.execute_wait environment wait handling, part of #3515 #3682

Merged
merged 2 commits into from
Nov 7, 2024

Conversation

benclifford
Copy link
Collaborator

Prior to this PR, Channel.execute_wait took a parameter to modify the unix environment of the newly executed process. This is unused in current Parsl. As part of the implementation of that, LocalChannel cached the unix environment at object initialization, and used that as the base to override with any supplied execute_wait environment parameter.

Post this PR:

The env parameter of execute_wait is removed.

LocalChannel does not cache the environment any more. The executed process will inherit the parent process environment as of the point of execution not object initialization.

Changed Behaviour

This is a behaviour change: if the workflow process changes its unix environment after creating the configuration objects, then prior to this PR, executed processes would not observe that change. Post this PR, executed processes will observe that change. I hope this is not a big deal.

Type of change

  • Code maintenance/cleanup

Prior to this PR, Channel.execute_wait took a parameter to modify the
unix environment of the newly executed process. This is unused in current
Parsl. As part of the implementation of that, LocalChannel cached the
unix environment at object initialization, and used that as the base to
override with any supplied execute_wait environment parameter.

Post this PR:

The `env` parameter of execute_wait is removed.

LocalChannel does not cache the environment any more. The executed process
will inherit the parent process environment as of the point of execution
not object initialization.

This is a behaviour change: if the workflow process changes its unix
environment after creating the configuration objects, then prior to this
PR, executed processes would not observe that change. Post this PR,
executed processes will observe that change. I hope this is not a big
deal.
Copy link
Collaborator

@khk-globus khk-globus left a comment

Choose a reason for hiding this comment

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

Hurrah. 🎉

And hah:

Post this PR, executed processes will observe that change. I hope this is not a big deal.

@benclifford benclifford enabled auto-merge November 7, 2024 14:45
@benclifford benclifford added this pull request to the merge queue Nov 7, 2024
Merged via the queue into master with commit 6683f5b Nov 7, 2024
7 checks passed
@benclifford benclifford deleted the benc-3515-localchannel-envs branch November 7, 2024 15:33
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.

2 participants