Skip to content

Commit

Permalink
Remove Channel.execute_wait environment wait handling, part of #3515 (#…
Browse files Browse the repository at this point in the history
…3682)

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
  • Loading branch information
benclifford authored Nov 7, 2024
1 parent 08aa003 commit 6683f5b
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 40 deletions.
7 changes: 2 additions & 5 deletions parsl/channels/base.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from abc import ABCMeta, abstractmethod, abstractproperty
from typing import Dict, Tuple
from typing import Tuple


class Channel(metaclass=ABCMeta):
Expand All @@ -22,16 +22,13 @@ class Channel(metaclass=ABCMeta):
"""

@abstractmethod
def execute_wait(self, cmd: str, walltime: int = 0, envs: Dict[str, str] = {}) -> Tuple[int, str, str]:
def execute_wait(self, cmd: str, walltime: int = 0) -> Tuple[int, str, str]:
''' Executes the cmd, with a defined walltime.
Args:
- cmd (string): Command string to execute over the channel
- walltime (int) : Timeout in seconds
KWargs:
- envs (Dict[str, str]) : Environment variables to push to the remote side
Returns:
- (exit_code, stdout, stderr) (int, string, string)
'''
Expand Down
18 changes: 2 additions & 16 deletions parsl/channels/local/local.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import copy
import logging
import os
import shutil
Expand All @@ -16,46 +15,33 @@ class LocalChannel(Channel, RepresentationMixin):
and done so infrequently that they do not need a persistent channel
'''

def __init__(self, envs={}, script_dir=None):
def __init__(self, script_dir=None):
''' Initialize the local channel. script_dir is required by set to a default.
KwArgs:
- envs (dict) : A dictionary of env variables to be set when launching the shell
- script_dir (string): Directory to place scripts
'''
self.hostname = "localhost"
self.envs = envs
local_env = os.environ.copy()
self._envs = copy.deepcopy(local_env)
self._envs.update(envs)
self.script_dir = script_dir

def execute_wait(self, cmd, walltime=None, envs={}):
def execute_wait(self, cmd, walltime=None):
''' Synchronously execute a commandline string on the shell.
Args:
- cmd (string) : Commandline string to execute
- walltime (int) : walltime in seconds
Kwargs:
- envs (dict) : Dictionary of env variables. This will be used
to override the envs set at channel initialization.
Returns:
- retcode : Return code from the execution
- stdout : stdout string
- stderr : stderr string
'''
current_env = copy.deepcopy(self._envs)
current_env.update(envs)

try:
logger.debug("Creating process with command '%s'", cmd)
proc = subprocess.Popen(
cmd,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
env=current_env,
shell=True,
preexec_fn=os.setpgrp
)
Expand Down
19 changes: 0 additions & 19 deletions parsl/tests/test_channels/test_local_channel.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,3 @@ def test_env():

x = [s for s in stdout if s.startswith("HOME=")]
assert x, "HOME not found"


@pytest.mark.local
def test_env_mod():
''' Testing for env update at execute time.
'''

lc = LocalChannel()
rc, stdout, stderr = lc.execute_wait("env", 1, {'TEST_ENV': 'fooo'})

stdout = stdout.split('\n')
x = [s for s in stdout if s.startswith("PATH=")]
assert x, "PATH not found"

x = [s for s in stdout if s.startswith("HOME=")]
assert x, "HOME not found"

x = [s for s in stdout if s.startswith("TEST_ENV=fooo")]
assert x, "User set env missing"

0 comments on commit 6683f5b

Please sign in to comment.