From 6c6f35c882579b331b8aee1085af6c2e7537d932 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Wed, 23 Oct 2024 14:11:59 +0000 Subject: [PATCH 01/71] Remove all ssh channels - passes 'make flake8 mypy isort' --- parsl/channels/oauth_ssh/__init__.py | 0 parsl/channels/oauth_ssh/oauth_ssh.py | 119 ------- parsl/channels/ssh/__init__.py | 0 parsl/channels/ssh/ssh.py | 306 ------------------ parsl/channels/ssh_il/__init__.py | 0 parsl/channels/ssh_il/ssh_il.py | 85 ----- .../test_providers/test_local_provider.py | 135 -------- 7 files changed, 645 deletions(-) delete mode 100644 parsl/channels/oauth_ssh/__init__.py delete mode 100644 parsl/channels/oauth_ssh/oauth_ssh.py delete mode 100644 parsl/channels/ssh/__init__.py delete mode 100644 parsl/channels/ssh/ssh.py delete mode 100644 parsl/channels/ssh_il/__init__.py delete mode 100644 parsl/channels/ssh_il/ssh_il.py diff --git a/parsl/channels/oauth_ssh/__init__.py b/parsl/channels/oauth_ssh/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/parsl/channels/oauth_ssh/oauth_ssh.py b/parsl/channels/oauth_ssh/oauth_ssh.py deleted file mode 100644 index 1b690a4e3c..0000000000 --- a/parsl/channels/oauth_ssh/oauth_ssh.py +++ /dev/null @@ -1,119 +0,0 @@ -import logging -import socket - -from parsl.channels.ssh.ssh import DeprecatedSSHChannel -from parsl.errors import OptionalModuleMissing - -try: - import paramiko - _ssh_enabled = True -except (ImportError, NameError, FileNotFoundError): - _ssh_enabled = False - -try: - from oauth_ssh.oauth_ssh_token import find_access_token - from oauth_ssh.ssh_service import SSHService - _oauth_ssh_enabled = True -except (ImportError, NameError): - _oauth_ssh_enabled = False - - -logger = logging.getLogger(__name__) - - -class DeprecatedOAuthSSHChannel(DeprecatedSSHChannel): - """SSH persistent channel. This enables remote execution on sites - accessible via ssh. This channel uses Globus based OAuth tokens for authentication. - """ - - def __init__(self, hostname, username=None, script_dir=None, envs=None, port=22): - ''' Initialize a persistent connection to the remote system. - We should know at this point whether ssh connectivity is possible - - Args: - - hostname (String) : Hostname - - KWargs: - - username (string) : Username on remote system - - script_dir (string) : Full path to a script dir where - generated scripts could be sent to. - - envs (dict) : A dictionary of env variables to be set when executing commands - - port (int) : Port at which the SSHService is running - - Raises: - ''' - if not _ssh_enabled: - raise OptionalModuleMissing(['ssh'], - "OauthSSHChannel requires the ssh module and config.") - - if not _oauth_ssh_enabled: - raise OptionalModuleMissing(['oauth_ssh'], - "OauthSSHChannel requires oauth_ssh module and config.") - - self.hostname = hostname - self.username = username - self.script_dir = script_dir - self.port = port - self.envs = {} - if envs is not None: - self.envs = envs - - try: - access_token = find_access_token(hostname) - except Exception: - logger.exception("Failed to find the access token for {}".format(hostname)) - raise - - try: - self.service = SSHService(hostname, port) - self.transport = self.service.login(access_token, username) - - except Exception: - logger.exception("Caught an exception in the OAuth authentication step with {}".format(hostname)) - raise - - self.sftp_client = paramiko.SFTPClient.from_transport(self.transport) - - def execute_wait(self, cmd, walltime=60, envs={}): - ''' Synchronously execute a commandline string on the shell. - - This command does *NOT* honor walltime currently. - - Args: - - cmd (string) : Commandline string to execute - - Kwargs: - - walltime (int) : walltime in seconds - - envs (dict) : Dictionary of env variables - - Returns: - - retcode : Return code from the execution, -1 on fail - - stdout : stdout string - - stderr : stderr string - - Raises: - None. - ''' - - session = self.transport.open_session() - session.setblocking(0) - - nbytes = 1024 - session.exec_command(self.prepend_envs(cmd, envs)) - session.settimeout(walltime) - - try: - # Wait until command is executed - exit_status = session.recv_exit_status() - - stdout = session.recv(nbytes).decode('utf-8') - stderr = session.recv_stderr(nbytes).decode('utf-8') - - except socket.timeout: - logger.exception("Command failed to execute without timeout limit on {}".format(self)) - raise - - return exit_status, stdout, stderr - - def close(self) -> None: - self.transport.close() diff --git a/parsl/channels/ssh/__init__.py b/parsl/channels/ssh/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/parsl/channels/ssh/ssh.py b/parsl/channels/ssh/ssh.py deleted file mode 100644 index c53a26b831..0000000000 --- a/parsl/channels/ssh/ssh.py +++ /dev/null @@ -1,306 +0,0 @@ -import errno -import logging -import os - -from parsl.channels.base import Channel -from parsl.channels.errors import ( - AuthException, - BadHostKeyException, - BadPermsScriptPath, - BadScriptPath, - FileCopyException, - SSHException, -) -from parsl.errors import OptionalModuleMissing -from parsl.utils import RepresentationMixin - -try: - import paramiko - _ssh_enabled = True -except (ImportError, NameError, FileNotFoundError): - _ssh_enabled = False - - -logger = logging.getLogger(__name__) - - -if _ssh_enabled: - class NoAuthSSHClient(paramiko.SSHClient): - def _auth(self, username, *args): - self._transport.auth_none(username) - return - - -class DeprecatedSSHChannel(Channel, RepresentationMixin): - ''' SSH persistent channel. This enables remote execution on sites - accessible via ssh. It is assumed that the user has setup host keys - so as to ssh to the remote host. Which goes to say that the following - test on the commandline should work: - - >>> ssh @ - - ''' - - def __init__(self, hostname, username=None, password=None, script_dir=None, envs=None, - gssapi_auth=False, skip_auth=False, port=22, key_filename=None, host_keys_filename=None): - ''' Initialize a persistent connection to the remote system. - We should know at this point whether ssh connectivity is possible - - Args: - - hostname (String) : Hostname - - KWargs: - - username (string) : Username on remote system - - password (string) : Password for remote system - - port : The port designated for the ssh connection. Default is 22. - - script_dir (string) : Full path to a script dir where - generated scripts could be sent to. - - envs (dict) : A dictionary of environment variables to be set when executing commands - - key_filename (string or list): the filename, or list of filenames, of optional private key(s) - - Raises: - ''' - if not _ssh_enabled: - raise OptionalModuleMissing(['ssh'], - "SSHChannel requires the ssh module and config.") - - self.hostname = hostname - self.username = username - self.password = password - self.port = port - self.script_dir = script_dir - self.skip_auth = skip_auth - self.gssapi_auth = gssapi_auth - self.key_filename = key_filename - self.host_keys_filename = host_keys_filename - - if self.skip_auth: - self.ssh_client = NoAuthSSHClient() - else: - self.ssh_client = paramiko.SSHClient() - self.ssh_client.load_system_host_keys(filename=host_keys_filename) - self.ssh_client.set_missing_host_key_policy(paramiko.AutoAddPolicy()) - self.sftp_client = None - - self.envs = {} - if envs is not None: - self.envs = envs - - def _is_connected(self): - transport = self.ssh_client.get_transport() if self.ssh_client else None - return transport and transport.is_active() - - def _connect(self): - if not self._is_connected(): - logger.debug(f"connecting to {self.hostname}:{self.port}") - try: - self.ssh_client.connect( - self.hostname, - username=self.username, - password=self.password, - port=self.port, - allow_agent=True, - gss_auth=self.gssapi_auth, - gss_kex=self.gssapi_auth, - key_filename=self.key_filename - ) - transport = self.ssh_client.get_transport() - self.sftp_client = paramiko.SFTPClient.from_transport(transport) - - except paramiko.BadHostKeyException as e: - raise BadHostKeyException(e, self.hostname) - - except paramiko.AuthenticationException as e: - raise AuthException(e, self.hostname) - - except paramiko.SSHException as e: - raise SSHException(e, self.hostname) - - except Exception as e: - raise SSHException(e, self.hostname) - - def _valid_sftp_client(self): - self._connect() - return self.sftp_client - - def _valid_ssh_client(self): - self._connect() - return self.ssh_client - - def prepend_envs(self, cmd, env={}): - env.update(self.envs) - - if len(env.keys()) > 0: - env_vars = ' '.join(['{}={}'.format(key, value) for key, value in env.items()]) - return 'env {0} {1}'.format(env_vars, cmd) - return cmd - - def execute_wait(self, cmd, walltime=2, envs={}): - ''' 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 - - Returns: - - retcode : Return code from the execution, -1 on fail - - stdout : stdout string - - stderr : stderr string - - Raises: - None. - ''' - - # Execute the command - stdin, stdout, stderr = self._valid_ssh_client().exec_command( - self.prepend_envs(cmd, envs), bufsize=-1, timeout=walltime - ) - # Block on exit status from the command - exit_status = stdout.channel.recv_exit_status() - return exit_status, stdout.read().decode("utf-8"), stderr.read().decode("utf-8") - - def push_file(self, local_source, remote_dir): - ''' Transport a local file to a directory on a remote machine - - Args: - - local_source (string): Path - - remote_dir (string): Remote path - - Returns: - - str: Path to copied file on remote machine - - Raises: - - BadScriptPath : if script path on the remote side is bad - - BadPermsScriptPath : You do not have perms to make the channel script dir - - FileCopyException : FileCopy failed. - - ''' - remote_dest = os.path.join(remote_dir, os.path.basename(local_source)) - - try: - self.makedirs(remote_dir, exist_ok=True) - except IOError as e: - logger.exception("Pushing {0} to {1} failed".format(local_source, remote_dir)) - if e.errno == 2: - raise BadScriptPath(e, self.hostname) - elif e.errno == 13: - raise BadPermsScriptPath(e, self.hostname) - else: - logger.exception("File push failed due to SFTP client failure") - raise FileCopyException(e, self.hostname) - try: - self._valid_sftp_client().put(local_source, remote_dest, confirm=True) - # Set perm because some systems require the script to be executable - self._valid_sftp_client().chmod(remote_dest, 0o700) - except Exception as e: - logger.exception("File push from local source {} to remote destination {} failed".format( - local_source, remote_dest)) - raise FileCopyException(e, self.hostname) - - return remote_dest - - def pull_file(self, remote_source, local_dir): - ''' Transport file on the remote side to a local directory - - Args: - - remote_source (string): remote_source - - local_dir (string): Local directory to copy to - - - Returns: - - str: Local path to file - - Raises: - - FileExists : Name collision at local directory. - - FileCopyException : FileCopy failed. - ''' - - local_dest = os.path.join(local_dir, os.path.basename(remote_source)) - - try: - os.makedirs(local_dir) - except OSError as e: - if e.errno != errno.EEXIST: - logger.exception("Failed to create local_dir: {0}".format(local_dir)) - raise BadScriptPath(e, self.hostname) - - try: - self._valid_sftp_client().get(remote_source, local_dest) - except Exception as e: - logger.exception("File pull failed") - raise FileCopyException(e, self.hostname) - - return local_dest - - def close(self) -> None: - if self._is_connected(): - transport = self.ssh_client.get_transport() - self.ssh_client.close() - - # ssh_client.close calls transport.close, but transport.close does - # not always wait for the transport thread to be stopped. See impl - # of Transport.close in paramiko and issue - # https://github.com/paramiko/paramiko/issues/520 - logger.debug("Waiting for transport thread to stop") - transport.join(30) - if transport.is_alive(): - logger.warning("SSH transport thread did not shut down") - else: - logger.debug("SSH transport thread stopped") - - def isdir(self, path): - """Return true if the path refers to an existing directory. - - Parameters - ---------- - path : str - Path of directory on the remote side to check. - """ - result = True - try: - self._valid_sftp_client().lstat(path) - except FileNotFoundError: - result = False - - return result - - def makedirs(self, path, mode=0o700, exist_ok=False): - """Create a directory on the remote side. - - If intermediate directories do not exist, they will be created. - - Parameters - ---------- - path : str - Path of directory on the remote side to create. - mode : int - Permissions (posix-style) for the newly-created directory. - exist_ok : bool - If False, raise an OSError if the target directory already exists. - """ - if exist_ok is False and self.isdir(path): - raise OSError('Target directory {} already exists'.format(path)) - - self.execute_wait('mkdir -p {}'.format(path)) - self._valid_sftp_client().chmod(path, mode) - - def abspath(self, path): - """Return the absolute path on the remote side. - - Parameters - ---------- - path : str - Path for which the absolute path will be returned. - """ - return self._valid_sftp_client().normalize(path) - - @property - def script_dir(self): - return self._script_dir - - @script_dir.setter - def script_dir(self, value): - self._script_dir = value diff --git a/parsl/channels/ssh_il/__init__.py b/parsl/channels/ssh_il/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/parsl/channels/ssh_il/ssh_il.py b/parsl/channels/ssh_il/ssh_il.py deleted file mode 100644 index 67e5501a43..0000000000 --- a/parsl/channels/ssh_il/ssh_il.py +++ /dev/null @@ -1,85 +0,0 @@ -import getpass -import logging - -from parsl.channels.ssh.ssh import DeprecatedSSHChannel -from parsl.errors import OptionalModuleMissing - -try: - import paramiko - _ssh_enabled = True -except (ImportError, NameError, FileNotFoundError): - _ssh_enabled = False - - -logger = logging.getLogger(__name__) - - -class DeprecatedSSHInteractiveLoginChannel(DeprecatedSSHChannel): - """SSH persistent channel. This enables remote execution on sites - accessible via ssh. This channel supports interactive login and is appropriate when - keys are not set up. - """ - - def __init__(self, hostname, username=None, password=None, script_dir=None, envs=None): - ''' Initialize a persistent connection to the remote system. - We should know at this point whether ssh connectivity is possible - - Args: - - hostname (String) : Hostname - - KWargs: - - username (string) : Username on remote system - - password (string) : Password for remote system - - script_dir (string) : Full path to a script dir where - generated scripts could be sent to. - - envs (dict) : A dictionary of env variables to be set when executing commands - - Raises: - ''' - if not _ssh_enabled: - raise OptionalModuleMissing(['ssh'], - "SSHInteractiveLoginChannel requires the ssh module and config.") - - self.hostname = hostname - self.username = username - self.password = password - - self.ssh_client = paramiko.SSHClient() - self.ssh_client.load_system_host_keys() - self.ssh_client.set_missing_host_key_policy(paramiko.AutoAddPolicy()) - - self.script_dir = script_dir - - self.envs = {} - if envs is not None: - self.envs = envs - - try: - self.ssh_client.connect( - hostname, username=username, password=password, allow_agent=True - ) - - except Exception: - logger.debug("Caught the SSHException in SSHInteractive") - pass - ''' - except paramiko.BadHostKeyException as e: - raise BadHostKeyException(e, self.hostname) - - except paramiko.AuthenticationException as e: - raise AuthException(e, self.hostname) - - except paramiko.SSHException as e: - logger.debug("Caught the SSHException in SSHInteractive") - pass - - except Exception as e: - raise SSHException(e, self.hostname) - ''' - - transport = self.ssh_client.get_transport() - - il_password = getpass.getpass('Enter {0} Logon password :'.format(hostname)) - transport.auth_password(username, il_password) - - self.sftp_client = paramiko.SFTPClient.from_transport(transport) diff --git a/parsl/tests/test_providers/test_local_provider.py b/parsl/tests/test_providers/test_local_provider.py index 497c13370d..8d56d89ddf 100644 --- a/parsl/tests/test_providers/test_local_provider.py +++ b/parsl/tests/test_providers/test_local_provider.py @@ -12,7 +12,6 @@ import pytest from parsl.channels import LocalChannel -from parsl.channels.ssh.ssh import DeprecatedSSHChannel from parsl.jobs.states import JobState from parsl.launchers import SingleNodeLauncher from parsl.providers import LocalProvider @@ -69,140 +68,6 @@ def test_local_channel(): _run_tests(p) -SSHD_CONFIG = """ -Port {port} -ListenAddress 127.0.0.1 -HostKey {hostkey} -AuthorizedKeysFile {connpubkey} -AuthenticationMethods publickey -StrictModes no -Subsystem sftp {sftp_path} -""" - - -# It would probably be better, when more formalized site testing comes into existence, to -# use a site-testing provided server/configuration instead of the current scheme -@pytest.mark.local -@pytest.mark.sshd_required -def test_ssh_channel(): - with tempfile.TemporaryDirectory() as config_dir: - sshd_thread, priv_key, server_port = _start_sshd(config_dir) - try: - with tempfile.TemporaryDirectory() as remote_script_dir: - # The SSH library fails to add the new host key to the file if the file does not - # already exist, so create it here. - pathlib.Path('{}/known.hosts'.format(config_dir)).touch(mode=0o600) - script_dir = tempfile.mkdtemp() - channel = DeprecatedSSHChannel('127.0.0.1', port=server_port, - script_dir=remote_script_dir, - host_keys_filename='{}/known.hosts'.format(config_dir), - key_filename=priv_key) - try: - p = LocalProvider(channel=channel, - launcher=SingleNodeLauncher(debug=False)) - p.script_dir = script_dir - _run_tests(p) - finally: - channel.close() - finally: - _stop_sshd(sshd_thread) - - -def _stop_sshd(sshd_thread): - sshd_thread.stop() - sshd_thread.join() - - -class SSHDThread(threading.Thread): - def __init__(self, config_file): - threading.Thread.__init__(self, daemon=True) - self.config_file = config_file - self.stop_flag = False - self.error = None - - def run(self): - try: - # sshd needs to be run with an absolute path, hence the call to which() - sshpath = shutil.which('sshd') - assert sshpath is not None, "can find sshd executable" - p = subprocess.Popen([sshpath, '-D', '-f', self.config_file], - stdout=subprocess.PIPE, stderr=subprocess.PIPE) - while True: - ec = p.poll() - if self.stop_flag: - p.terminate() - break - elif ec is None: - time.sleep(0.1) - elif ec == 0: - self.error = Exception('sshd exited prematurely: {}{}'.format(p.stdout.read(), - p.stderr.read())) - break - else: - self.error = Exception('sshd failed: {}{}'.format(p.stdout.read(), - p.stderr.read())) - break - except Exception as ex: - logger.exception("SSHDThread exception from run loop") - self.error = ex - - def stop(self): - self.stop_flag = True - - -def _start_sshd(config_dir: str): - server_config, priv_key, port = _init_sshd(config_dir) - sshd_thread = SSHDThread(server_config) - sshd_thread.start() - time.sleep(1.0) - if not sshd_thread.is_alive(): - raise Exception('Failed to start sshd: {}'.format(sshd_thread.error)) - return sshd_thread, priv_key, port - - -def _init_sshd(config_dir): - hostkey = '{}/hostkey'.format(config_dir) - connkey = '{}/connkey'.format(config_dir) - os.system('ssh-keygen -b 2048 -t rsa -q -N "" -f {}'.format(hostkey)) - os.system('ssh-keygen -b 2048 -t rsa -q -N "" -f {}'.format(connkey)) - port = _find_free_port(22222) - server_config_str = SSHD_CONFIG.format(port=port, hostkey=hostkey, - connpubkey='{}.pub'.format(connkey), - sftp_path=_get_system_sftp_path()) - server_config = '{}/sshd_config'.format(config_dir) - with open(server_config, 'w') as f: - f.write(server_config_str) - return server_config, connkey, port - - -def _get_system_sftp_path(): - try: - with open('/etc/ssh/sshd_config') as f: - line = f.readline() - while line: - tokens = line.split() - if tokens[0] == 'Subsystem' and tokens[1] == 'sftp': - return tokens[2] - line = f.readline() - except Exception: - pass - return '/usr/lib/openssh/sftp-server' - - -def _find_free_port(start: int): - port = start - while port < 65535: - s = socket.socket(socket.AF_INET, socket.SOCK_STREAM) - try: - s.bind(('127.0.0.1', port)) - s.close() - return port - except Exception: - pass - port += random.randint(1, 20) - raise Exception('Could not find free port') - - def _run(p: LocalProvider, command: str, np: int = 1): id = p.submit(command, np) return _wait(p, id) From f2cc4f050d3a497691de465887056731d054221a Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Wed, 23 Oct 2024 14:13:09 +0000 Subject: [PATCH 02/71] Remove unused bad host key exception --- docs/reference.rst | 1 - parsl/channels/errors.py | 15 --------------- 2 files changed, 16 deletions(-) diff --git a/docs/reference.rst b/docs/reference.rst index 3436635cad..c5411d7387 100644 --- a/docs/reference.rst +++ b/docs/reference.rst @@ -171,7 +171,6 @@ Exceptions parsl.providers.errors.SchedulerMissingArgs parsl.providers.errors.ScriptPathError parsl.channels.errors.ChannelError - parsl.channels.errors.BadHostKeyException parsl.channels.errors.BadScriptPath parsl.channels.errors.BadPermsScriptPath parsl.channels.errors.FileExists diff --git a/parsl/channels/errors.py b/parsl/channels/errors.py index 0358dc3ece..073ecf48c0 100644 --- a/parsl/channels/errors.py +++ b/parsl/channels/errors.py @@ -19,21 +19,6 @@ def __str__(self) -> str: return "Hostname:{0}, Reason:{1}".format(self.hostname, self.reason) -class BadHostKeyException(ChannelError): - ''' SSH channel could not be created since server's host keys could not - be verified - - Contains: - reason(string) - e (paramiko exception object) - hostname (string) - ''' - - def __init__(self, e: Exception, hostname: str) -> None: - super().__init__("SSH channel could not be created since server's host keys could not be " - "verified", e, hostname) - - class BadScriptPath(ChannelError): ''' An error raised during execution of an app. What this exception contains depends entirely on context From 55d7b8f811dcf00275eca41ce5b6acd2ac7d2d45 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Wed, 23 Oct 2024 14:13:55 +0000 Subject: [PATCH 03/71] Remove SSHException --- docs/reference.rst | 1 - parsl/channels/errors.py | 13 ------------- 2 files changed, 14 deletions(-) diff --git a/docs/reference.rst b/docs/reference.rst index c5411d7387..60dc6733f5 100644 --- a/docs/reference.rst +++ b/docs/reference.rst @@ -175,7 +175,6 @@ Exceptions parsl.channels.errors.BadPermsScriptPath parsl.channels.errors.FileExists parsl.channels.errors.AuthException - parsl.channels.errors.SSHException parsl.channels.errors.FileCopyException parsl.executors.high_throughput.errors.WorkerLost parsl.executors.high_throughput.interchange.ManagerLost diff --git a/parsl/channels/errors.py b/parsl/channels/errors.py index 073ecf48c0..9a6b19c134 100644 --- a/parsl/channels/errors.py +++ b/parsl/channels/errors.py @@ -73,19 +73,6 @@ def __init__(self, e: Exception, hostname: str) -> None: super().__init__("Authentication to remote server failed", e, hostname) -class SSHException(ChannelError): - ''' if there was any other error connecting or establishing an SSH session - - Contains: - reason(string) - e (paramiko exception object) - hostname (string) - ''' - - def __init__(self, e: Exception, hostname: str) -> None: - super().__init__("Error connecting or establishing an SSH session", e, hostname) - - class FileCopyException(ChannelError): ''' File copy operation failed From 99941fe546c15da31fb8dba5ea4715e7df0ebc98 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Wed, 23 Oct 2024 14:15:43 +0000 Subject: [PATCH 04/71] Remove a 'yet' that is not supported by an issue or roadmap item --- docs/userguide/configuring.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/userguide/configuring.rst b/docs/userguide/configuring.rst index a57e815fe7..be9a0efd22 100644 --- a/docs/userguide/configuring.rst +++ b/docs/userguide/configuring.rst @@ -492,7 +492,7 @@ CC-IN2P3 .. image:: https://cc.in2p3.fr/wp-content/uploads/2017/03/bandeau_accueil.jpg The snippet below shows an example configuration for executing from a login node on IN2P3's Computing Centre. -The configuration uses the `parsl.providers.LocalProvider` to run on a login node primarily to avoid GSISSH, which Parsl does not support yet. +The configuration uses the `parsl.providers.LocalProvider` to run on a login node primarily to avoid GSISSH, which Parsl does not support. This system uses Grid Engine which Parsl interfaces with using the `parsl.providers.GridEngineProvider`. .. literalinclude:: ../../parsl/configs/cc_in2p3.py From 6782deb1e7ba51c13f5842dec7c99666fac9f2ec Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Wed, 23 Oct 2024 14:18:22 +0000 Subject: [PATCH 05/71] Remove description from channel base class --- parsl/channels/base.py | 22 +++------------------- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/parsl/channels/base.py b/parsl/channels/base.py index 18a573d0e6..f650e7df3a 100644 --- a/parsl/channels/base.py +++ b/parsl/channels/base.py @@ -3,25 +3,9 @@ class Channel(metaclass=ABCMeta): - """Channels are abstractions that enable ExecutionProviders to talk to - resource managers of remote compute facilities. - - For certain resources such as campus clusters or supercomputers at - research laboratories, resource requirements may require authentication. - For instance some resources may allow access to their job schedulers from - only their login-nodes which require you to authenticate through SSH, or - require two factor authentication. - - The simplest Channel, *LocalChannel*, executes commands locally in a - shell, while the *SSHChannel* authenticates you to remote systems. - - Channels provide the ability to execute commands remotely, using the - execute_wait method, and manipulate the remote file system using methods - such as push_file, pull_file and makedirs. - - Channels should ensure that each launched command runs in a new process - group, so that providers (such as AdHocProvider and LocalProvider) which - terminate long running commands using process groups can do so. + """This is a base class for a feature that has now been removed from + Parsl. This class should go away / be merged downwards into + LocalChannel. """ @abstractmethod From 48285cca6f34f2a9ead167d7dbc47d01131dcb8b Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Wed, 23 Oct 2024 14:19:08 +0000 Subject: [PATCH 06/71] remove sshd_required test marker that has no tests any more --- parsl/tests/conftest.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/parsl/tests/conftest.py b/parsl/tests/conftest.py index b8af73e4bf..03c9a396d0 100644 --- a/parsl/tests/conftest.py +++ b/parsl/tests/conftest.py @@ -143,10 +143,6 @@ def pytest_configure(config): 'markers', 'staging_required: Marks tests that require a staging provider, when there is no sharedFS' ) - config.addinivalue_line( - 'markers', - 'sshd_required: Marks tests that require a SSHD' - ) config.addinivalue_line( 'markers', 'multiple_cores_required: Marks tests that require multiple cores, such as htex affinity' From 0833f16553c8bd090e8b6b686db201eb25f8a954 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Wed, 23 Oct 2024 14:19:40 +0000 Subject: [PATCH 07/71] Remove unneeded setup targets --- setup.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/setup.py b/setup.py index b381ecfc2b..cace8c0252 100755 --- a/setup.py +++ b/setup.py @@ -21,7 +21,6 @@ ], 'aws' : ['boto3'], 'kubernetes' : ['kubernetes'], - 'oauth_ssh' : ['oauth-ssh>=0.9'], 'docs' : [ 'ipython<=8.6.0', 'nbsphinx', @@ -35,7 +34,6 @@ 'flux': ['pyyaml', 'cffi', 'jsonschema'], 'proxystore': ['proxystore'], 'radical-pilot': ['radical.pilot==1.60', 'radical.utils==1.60'], - 'ssh': ['paramiko'], # Disabling psi-j since github direct links are not allowed by pypi # 'psij': ['psi-j-parsl@git+https://github.com/ExaWorks/psi-j-parsl'] } From 835a735d40d94295cb35a1ec34795372c2c851ac Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Wed, 23 Oct 2024 14:32:33 +0000 Subject: [PATCH 08/71] Remove channel docstrings from all providers --- parsl/providers/cluster_provider.py | 5 ----- parsl/providers/cobalt/cobalt.py | 5 ----- parsl/providers/condor/condor.py | 5 ----- parsl/providers/grid_engine/grid_engine.py | 5 ----- parsl/providers/lsf/lsf.py | 5 ----- parsl/providers/pbspro/pbspro.py | 5 ----- parsl/providers/slurm/slurm.py | 5 ----- parsl/providers/torque/torque.py | 5 ----- 8 files changed, 40 deletions(-) diff --git a/parsl/providers/cluster_provider.py b/parsl/providers/cluster_provider.py index 50f38a6136..27e0c57e88 100644 --- a/parsl/providers/cluster_provider.py +++ b/parsl/providers/cluster_provider.py @@ -17,11 +17,6 @@ class ClusterProvider(ExecutionProvider): ---------- label : str Label for this provider. - channel : Channel - Channel for accessing this provider. Possible channels include - :class:`~parsl.channels.LocalChannel` (the default), - :class:`~parsl.channels.SSHChannel`, or - :class:`~parsl.channels.SSHInteractiveLoginChannel`. walltime : str Walltime requested per block in HH:MM:SS. launcher : Launcher diff --git a/parsl/providers/cobalt/cobalt.py b/parsl/providers/cobalt/cobalt.py index 4039dfcbea..4cbdde90be 100644 --- a/parsl/providers/cobalt/cobalt.py +++ b/parsl/providers/cobalt/cobalt.py @@ -33,11 +33,6 @@ class CobaltProvider(ClusterProvider, RepresentationMixin): Parameters ---------- - channel : Channel - Channel for accessing this provider. Possible channels include - :class:`~parsl.channels.LocalChannel` (the default), - :class:`~parsl.channels.SSHChannel`, or - :class:`~parsl.channels.SSHInteractiveLoginChannel`. nodes_per_block : int Nodes to provision per block. min_blocks : int diff --git a/parsl/providers/condor/condor.py b/parsl/providers/condor/condor.py index 25266d3a2e..15734ee871 100644 --- a/parsl/providers/condor/condor.py +++ b/parsl/providers/condor/condor.py @@ -36,11 +36,6 @@ class CondorProvider(RepresentationMixin, ClusterProvider): Parameters ---------- - channel : Channel - Channel for accessing this provider. Possible channels include - :class:`~parsl.channels.LocalChannel` (the default), - :class:`~parsl.channels.SSHChannel`, or - :class:`~parsl.channels.SSHInteractiveLoginChannel`. nodes_per_block : int Nodes to provision per block. cores_per_slot : int diff --git a/parsl/providers/grid_engine/grid_engine.py b/parsl/providers/grid_engine/grid_engine.py index 8e231637a9..0e68aaaea2 100644 --- a/parsl/providers/grid_engine/grid_engine.py +++ b/parsl/providers/grid_engine/grid_engine.py @@ -36,11 +36,6 @@ class GridEngineProvider(ClusterProvider, RepresentationMixin): Parameters ---------- - channel : Channel - Channel for accessing this provider. Possible channels include - :class:`~parsl.channels.LocalChannel` (the default), - :class:`~parsl.channels.SSHChannel`, or - :class:`~parsl.channels.SSHInteractiveLoginChannel`. nodes_per_block : int Nodes to provision per block. min_blocks : int diff --git a/parsl/providers/lsf/lsf.py b/parsl/providers/lsf/lsf.py index c993b58c7b..2eaff23c3e 100644 --- a/parsl/providers/lsf/lsf.py +++ b/parsl/providers/lsf/lsf.py @@ -32,11 +32,6 @@ class LSFProvider(ClusterProvider, RepresentationMixin): Parameters ---------- - channel : Channel - Channel for accessing this provider. Possible channels include - :class:`~parsl.channels.LocalChannel` (the default), - :class:`~parsl.channels.SSHChannel`, or - :class:`~parsl.channels.SSHInteractiveLoginChannel`. nodes_per_block : int Nodes to provision per block. When request_by_nodes is False, it is computed by cores_per_block / cores_per_node. diff --git a/parsl/providers/pbspro/pbspro.py b/parsl/providers/pbspro/pbspro.py index c998602152..e7a1bb4a30 100644 --- a/parsl/providers/pbspro/pbspro.py +++ b/parsl/providers/pbspro/pbspro.py @@ -17,11 +17,6 @@ class PBSProProvider(TorqueProvider): Parameters ---------- - channel : Channel - Channel for accessing this provider. Possible channels include - :class:`~parsl.channels.LocalChannel` (the default), - :class:`~parsl.channels.SSHChannel`, or - :class:`~parsl.channels.SSHInteractiveLoginChannel`. account : str Account the job will be charged against. queue : str diff --git a/parsl/providers/slurm/slurm.py b/parsl/providers/slurm/slurm.py index 54b4053fed..43719f9da5 100644 --- a/parsl/providers/slurm/slurm.py +++ b/parsl/providers/slurm/slurm.py @@ -70,11 +70,6 @@ class SlurmProvider(ClusterProvider, RepresentationMixin): Slurm queue to place job in. If unspecified or ``None``, no queue slurm directive will be specified. constraint : str Slurm job constraint, often used to choose cpu or gpu type. If unspecified or ``None``, no constraint slurm directive will be added. - channel : Channel - Channel for accessing this provider. Possible channels include - :class:`~parsl.channels.LocalChannel` (the default), - :class:`~parsl.channels.SSHChannel`, or - :class:`~parsl.channels.SSHInteractiveLoginChannel`. nodes_per_block : int Nodes to provision per block. cores_per_node : int diff --git a/parsl/providers/torque/torque.py b/parsl/providers/torque/torque.py index dfe267dbd3..aa030a60f6 100644 --- a/parsl/providers/torque/torque.py +++ b/parsl/providers/torque/torque.py @@ -33,11 +33,6 @@ class TorqueProvider(ClusterProvider, RepresentationMixin): Parameters ---------- - channel : Channel - Channel for accessing this provider. Possible channels include - :class:`~parsl.channels.LocalChannel` (the default), - :class:`~parsl.channels.SSHChannel`, or - :class:`~parsl.channels.SSHInteractiveLoginChannel`. account : str Account the job will be charged against. queue : str From 1af5a1ffa10fa70c4c2c1e5605f2b760794574ee Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Wed, 23 Oct 2024 14:34:28 +0000 Subject: [PATCH 09/71] Remove ad-hoc provider --- parsl/providers/ad_hoc/__init__.py | 0 parsl/providers/ad_hoc/ad_hoc.py | 252 -------------------------- parsl/tests/configs/local_adhoc.py | 18 -- parsl/tests/sites/test_local_adhoc.py | 62 ------- 4 files changed, 332 deletions(-) delete mode 100644 parsl/providers/ad_hoc/__init__.py delete mode 100644 parsl/providers/ad_hoc/ad_hoc.py delete mode 100644 parsl/tests/configs/local_adhoc.py delete mode 100644 parsl/tests/sites/test_local_adhoc.py diff --git a/parsl/providers/ad_hoc/__init__.py b/parsl/providers/ad_hoc/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/parsl/providers/ad_hoc/ad_hoc.py b/parsl/providers/ad_hoc/ad_hoc.py deleted file mode 100644 index 9059648101..0000000000 --- a/parsl/providers/ad_hoc/ad_hoc.py +++ /dev/null @@ -1,252 +0,0 @@ -import logging -import os -import time - -from parsl.channels import LocalChannel -from parsl.jobs.states import JobState, JobStatus -from parsl.launchers import SimpleLauncher -from parsl.providers.base import ExecutionProvider -from parsl.providers.errors import ScriptPathError -from parsl.utils import RepresentationMixin - -logger = logging.getLogger(__name__) - - -class DeprecatedAdHocProvider(ExecutionProvider, RepresentationMixin): - """ Deprecated ad-hoc execution provider - - The (former) AdHocProvider is deprecated. See - `issue #3515 `_ - for further discussion. - - This provider is used to provision execution resources over one or more ad hoc nodes - that are each accessible over a Channel (say, ssh) but otherwise lack a cluster scheduler. - - Parameters - ---------- - - channels : list of Channel ojects - Each channel represents a connection to a remote node - - worker_init : str - Command to be run before starting a worker, such as 'module load Anaconda; source activate env'. - Since this provider calls the same worker_init across all nodes in the ad-hoc cluster, it is - recommended that a single script is made available across nodes such as ~/setup.sh that can - be invoked. - - cmd_timeout : int - Duration for which the provider will wait for a command to be invoked on a remote system. - Defaults to 30s - - parallelism : float - Determines the ratio of workers to tasks as managed by the strategy component - - """ - - def __init__(self, - channels=[], - worker_init='', - cmd_timeout=30, - parallelism=1, - move_files=None): - - self.channels = channels - self._label = 'ad-hoc' - self.worker_init = worker_init - self.cmd_timeout = cmd_timeout - self.parallelism = 1 - self.move_files = move_files - self.launcher = SimpleLauncher() - self.init_blocks = self.min_blocks = self.max_blocks = len(channels) - - # This will be overridden by the DFK to the rundirs. - self.script_dir = "." - - # In ad-hoc mode, nodes_per_block should be 1 - self.nodes_per_block = 1 - - # Dictionary that keeps track of jobs, keyed on job_id - self.resources = {} - - self.least_loaded = self._least_loaded() - logger.debug("AdHoc provider initialized") - - def _write_submit_script(self, script_string, script_filename): - ''' - Load the template string with config values and write the generated submit script to - a submit script file. - - Parameters - ---------- - script_string: (string) - The template string to be used for the writing submit script - - script_filename: (string) - Name of the submit script - - Returns - ------- - None: on success - - Raises - ------ - ScriptPathError - Unable to write submit script out - ''' - - try: - with open(script_filename, 'w') as f: - f.write(script_string) - - except IOError as e: - logger.error("Failed writing to submit script: %s", script_filename) - raise ScriptPathError(script_filename, e) - - return None - - def _least_loaded(self): - """ Find channels that are not in use - - Returns - ------- - channel : Channel object - None : When there are no more available channels - """ - while True: - channel_counts = {channel: 0 for channel in self.channels} - for job_id in self.resources: - channel = self.resources[job_id]['channel'] - if self.resources[job_id]['status'].state == JobState.RUNNING: - channel_counts[channel] = channel_counts.get(channel, 0) + 1 - else: - channel_counts[channel] = channel_counts.get(channel, 0) - - logger.debug("Channel_counts : {}".format(channel_counts)) - if 0 not in channel_counts.values(): - yield None - - for channel in channel_counts: - if channel_counts[channel] == 0: - yield channel - - def submit(self, command, tasks_per_node, job_name="parsl.adhoc"): - ''' Submits the command onto a channel from the list of channels - - Submit returns an ID that corresponds to the task that was just submitted. - - Parameters - ---------- - command: (String) - Commandline invocation to be made on the remote side. - - tasks_per_node: (int) - command invocations to be launched per node - - job_name: (String) - Name of the job. Default : parsl.adhoc - - - Returns - ------- - None - At capacity, cannot provision more - - job_id: (string) - Identifier for the job - - ''' - channel = next(self.least_loaded) - if channel is None: - logger.warning("All Channels in Ad-Hoc provider are in use") - return None - - job_name = "{0}.{1}".format(job_name, time.time()) - - # Set script path - script_path = "{0}/{1}.sh".format(self.script_dir, job_name) - script_path = os.path.abspath(script_path) - - wrap_command = self.worker_init + '\n' + self.launcher(command, tasks_per_node, self.nodes_per_block) - - self._write_submit_script(wrap_command, script_path) - - job_id = None - remote_pid = None - final_cmd = None - - if (self.move_files is None and not isinstance(channel, LocalChannel)) or (self.move_files): - logger.debug("Pushing start script") - script_path = channel.push_file(script_path, channel.script_dir) - - # Bash would return until the streams are closed. So we redirect to a outs file - final_cmd = 'bash {0} > {0}.out 2>&1 & \n echo "PID:$!" '.format(script_path) - retcode, stdout, stderr = channel.execute_wait(final_cmd, self.cmd_timeout) - for line in stdout.split('\n'): - if line.startswith("PID:"): - remote_pid = line.split("PID:")[1].strip() - job_id = remote_pid - if job_id is None: - logger.warning("Channel failed to start remote command/retrieve PID") - - self.resources[job_id] = {'job_id': job_id, - 'status': JobStatus(JobState.RUNNING), - 'cmd': final_cmd, - 'channel': channel, - 'remote_pid': remote_pid} - - return job_id - - def status(self, job_ids): - """ Get status of the list of jobs with job_ids - - Parameters - ---------- - job_ids : list of strings - List of job id strings - - Returns - ------- - list of JobStatus objects - """ - for job_id in job_ids: - channel = self.resources[job_id]['channel'] - status_command = "ps --pid {} | grep {}".format(self.resources[job_id]['job_id'], - self.resources[job_id]['cmd'].split()[0]) - retcode, stdout, stderr = channel.execute_wait(status_command) - if retcode != 0 and self.resources[job_id]['status'].state == JobState.RUNNING: - self.resources[job_id]['status'] = JobStatus(JobState.FAILED) - - return [self.resources[job_id]['status'] for job_id in job_ids] - - def cancel(self, job_ids): - """ Cancel a list of jobs with job_ids - - Parameters - ---------- - job_ids : list of strings - List of job id strings - - Returns - ------- - list of confirmation bools: [True, False...] - """ - logger.debug("Cancelling jobs: {}".format(job_ids)) - rets = [] - for job_id in job_ids: - channel = self.resources[job_id]['channel'] - cmd = "kill -TERM -$(ps -o pgid= {} | grep -o '[0-9]*')".format(self.resources[job_id]['job_id']) - retcode, stdout, stderr = channel.execute_wait(cmd) - if retcode == 0: - rets.append(True) - else: - rets.append(False) - self.resources[job_id]['status'] = JobStatus(JobState.COMPLETED) - return rets - - @property - def label(self): - return self._label - - @property - def status_polling_interval(self): - return 10 diff --git a/parsl/tests/configs/local_adhoc.py b/parsl/tests/configs/local_adhoc.py deleted file mode 100644 index 9b1f951842..0000000000 --- a/parsl/tests/configs/local_adhoc.py +++ /dev/null @@ -1,18 +0,0 @@ -from parsl.channels import LocalChannel -from parsl.config import Config -from parsl.executors import HighThroughputExecutor -from parsl.providers.ad_hoc.ad_hoc import DeprecatedAdHocProvider - - -def fresh_config(): - return Config( - executors=[ - HighThroughputExecutor( - label='AdHoc', - encrypted=True, - provider=DeprecatedAdHocProvider( - channels=[LocalChannel(), LocalChannel()] - ) - ) - ] - ) diff --git a/parsl/tests/sites/test_local_adhoc.py b/parsl/tests/sites/test_local_adhoc.py deleted file mode 100644 index bdb922773c..0000000000 --- a/parsl/tests/sites/test_local_adhoc.py +++ /dev/null @@ -1,62 +0,0 @@ -import logging - -import pytest - -from parsl import python_app -from parsl.tests.configs.local_adhoc import fresh_config as local_config - -logger = logging.getLogger(__name__) - - -@python_app -def python_app_2(): - import os - import threading - import time - time.sleep(1) - return "Hello from PID[{}] TID[{}]".format(os.getpid(), threading.current_thread()) - - -@python_app -def python_app_1(): - import os - import threading - import time - time.sleep(1) - return "Hello from PID[{}] TID[{}]".format(os.getpid(), threading.current_thread()) - - -@python_app -def bash_app(stdout=None, stderr=None): - return 'echo "Hello from $(uname -a)" ; sleep 2' - - -@pytest.mark.local -def test_python(N=2): - """Testing basic python functionality.""" - - r1 = {} - r2 = {} - for i in range(0, N): - r1[i] = python_app_1() - r2[i] = python_app_2() - print("Waiting ....") - - for x in r1: - print("python_app_1 : ", r1[x].result()) - for x in r2: - print("python_app_2 : ", r2[x].result()) - - return - - -@pytest.mark.local -def test_bash(): - """Testing basic bash functionality.""" - - import os - fname = os.path.basename(__file__) - - x = bash_app(stdout="{0}.out".format(fname)) - print("Waiting ....") - print(x.result()) From 9bdc90632d748c7e8e02ca0454c0213739bfcf6d Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Wed, 23 Oct 2024 14:46:15 +0000 Subject: [PATCH 10/71] Remove trivial/default LocalChannels from tests --- parsl/tests/configs/cc_in2p3.py | 2 -- parsl/tests/configs/frontera.py | 2 -- parsl/tests/configs/htex_local.py | 2 -- parsl/tests/configs/htex_local_alternate.py | 2 -- parsl/tests/configs/htex_local_intask_staging.py | 2 -- parsl/tests/configs/htex_local_rsync_staging.py | 2 -- parsl/tests/configs/slurm_local.py | 2 -- parsl/tests/manual_tests/htex_local.py | 2 -- parsl/tests/manual_tests/test_memory_limits.py | 2 -- parsl/tests/scaling_tests/htex_local.py | 2 -- parsl/tests/sites/test_affinity.py | 2 -- parsl/tests/sites/test_worker_info.py | 2 -- parsl/tests/test_htex/test_drain.py | 2 -- parsl/tests/test_htex/test_manager_selector_by_block.py | 2 -- .../test_monitoring/test_htex_init_blocks_vs_monitoring.py | 2 -- parsl/tests/test_providers/test_local_provider.py | 3 +-- parsl/tests/test_scaling/test_regression_1621.py | 2 -- parsl/tests/test_scaling/test_scale_down.py | 1 - parsl/tests/test_scaling/test_scale_down_htex_auto_scale.py | 2 -- parsl/tests/test_scaling/test_scale_down_htex_unregistered.py | 2 -- parsl/tests/test_scaling/test_shutdown_scalein.py | 2 -- 21 files changed, 1 insertion(+), 41 deletions(-) diff --git a/parsl/tests/configs/cc_in2p3.py b/parsl/tests/configs/cc_in2p3.py index 38c817cccd..5d9edf9d0c 100644 --- a/parsl/tests/configs/cc_in2p3.py +++ b/parsl/tests/configs/cc_in2p3.py @@ -1,4 +1,3 @@ -from parsl.channels import LocalChannel from parsl.config import Config from parsl.executors import HighThroughputExecutor from parsl.providers import GridEngineProvider @@ -14,7 +13,6 @@ def fresh_config(): max_workers_per_node=1, encrypted=True, provider=GridEngineProvider( - channel=LocalChannel(), nodes_per_block=2, init_blocks=2, max_blocks=2, diff --git a/parsl/tests/configs/frontera.py b/parsl/tests/configs/frontera.py index 537d6f78e0..41e2985e41 100644 --- a/parsl/tests/configs/frontera.py +++ b/parsl/tests/configs/frontera.py @@ -1,4 +1,3 @@ -from parsl.channels import LocalChannel from parsl.config import Config from parsl.executors import HighThroughputExecutor from parsl.launchers import SrunLauncher @@ -20,7 +19,6 @@ def fresh_config(): encrypted=True, provider=SlurmProvider( cmd_timeout=60, # Add extra time for slow scheduler responses - channel=LocalChannel(), nodes_per_block=2, init_blocks=1, min_blocks=1, diff --git a/parsl/tests/configs/htex_local.py b/parsl/tests/configs/htex_local.py index 5553a7f70a..17cccc090e 100644 --- a/parsl/tests/configs/htex_local.py +++ b/parsl/tests/configs/htex_local.py @@ -1,4 +1,3 @@ -from parsl.channels import LocalChannel from parsl.config import Config from parsl.executors import HighThroughputExecutor from parsl.launchers import SimpleLauncher @@ -14,7 +13,6 @@ def fresh_config(): cores_per_worker=1, encrypted=True, provider=LocalProvider( - channel=LocalChannel(), init_blocks=1, max_blocks=1, launcher=SimpleLauncher(), diff --git a/parsl/tests/configs/htex_local_alternate.py b/parsl/tests/configs/htex_local_alternate.py index d84a07ad84..ffff814b43 100644 --- a/parsl/tests/configs/htex_local_alternate.py +++ b/parsl/tests/configs/htex_local_alternate.py @@ -17,7 +17,6 @@ import os -from parsl.channels import LocalChannel from parsl.config import Config from parsl.data_provider.file_noop import NoOpFileStaging from parsl.data_provider.ftp import FTPInTaskStaging @@ -48,7 +47,6 @@ def fresh_config(): poll_period=100, encrypted=True, provider=LocalProvider( - channel=LocalChannel(), init_blocks=0, min_blocks=0, max_blocks=5, diff --git a/parsl/tests/configs/htex_local_intask_staging.py b/parsl/tests/configs/htex_local_intask_staging.py index eef88ea2e1..69649b2bcb 100644 --- a/parsl/tests/configs/htex_local_intask_staging.py +++ b/parsl/tests/configs/htex_local_intask_staging.py @@ -1,4 +1,3 @@ -from parsl.channels import LocalChannel from parsl.config import Config from parsl.data_provider.file_noop import NoOpFileStaging from parsl.data_provider.ftp import FTPInTaskStaging @@ -15,7 +14,6 @@ cores_per_worker=1, encrypted=True, provider=LocalProvider( - channel=LocalChannel(), init_blocks=1, max_blocks=1, launcher=SimpleLauncher(), diff --git a/parsl/tests/configs/htex_local_rsync_staging.py b/parsl/tests/configs/htex_local_rsync_staging.py index 044d2b9f89..d24b1da66b 100644 --- a/parsl/tests/configs/htex_local_rsync_staging.py +++ b/parsl/tests/configs/htex_local_rsync_staging.py @@ -1,4 +1,3 @@ -from parsl.channels import LocalChannel from parsl.config import Config from parsl.data_provider.ftp import FTPInTaskStaging from parsl.data_provider.http import HTTPInTaskStaging @@ -16,7 +15,6 @@ working_dir="./rsync-workdir/", encrypted=True, provider=LocalProvider( - channel=LocalChannel(), init_blocks=1, max_blocks=1, launcher=SimpleLauncher(), diff --git a/parsl/tests/configs/slurm_local.py b/parsl/tests/configs/slurm_local.py index 2a63f68e51..c0281cf340 100644 --- a/parsl/tests/configs/slurm_local.py +++ b/parsl/tests/configs/slurm_local.py @@ -1,4 +1,3 @@ -from parsl.channels import LocalChannel from parsl.config import Config from parsl.executors import HighThroughputExecutor from parsl.launchers import SrunLauncher @@ -13,7 +12,6 @@ def fresh_config(): encrypted=True, provider=SlurmProvider( cmd_timeout=60, # Add extra time for slow scheduler responses - channel=LocalChannel(), nodes_per_block=1, init_blocks=1, min_blocks=1, diff --git a/parsl/tests/manual_tests/htex_local.py b/parsl/tests/manual_tests/htex_local.py index bcfdbf34ec..e85da878b1 100644 --- a/parsl/tests/manual_tests/htex_local.py +++ b/parsl/tests/manual_tests/htex_local.py @@ -1,4 +1,3 @@ -from parsl.channels import LocalChannel from parsl.config import Config from parsl.executors import HighThroughputExecutor @@ -15,7 +14,6 @@ cores_per_worker=1, encrypted=True, provider=LocalProvider( - channel=LocalChannel(), init_blocks=1, max_blocks=1, # tasks_per_node=1, # For HighThroughputExecutor, this option should in most cases be 1 diff --git a/parsl/tests/manual_tests/test_memory_limits.py b/parsl/tests/manual_tests/test_memory_limits.py index b353ba0929..f98a779e82 100644 --- a/parsl/tests/manual_tests/test_memory_limits.py +++ b/parsl/tests/manual_tests/test_memory_limits.py @@ -5,7 +5,6 @@ import parsl from parsl.app.app import python_app # , bash_app -from parsl.channels import LocalChannel from parsl.config import Config from parsl.executors import HighThroughputExecutor from parsl.launchers import SingleNodeLauncher @@ -30,7 +29,6 @@ def test_simple(mem_per_worker): suppress_failure=True, encrypted=True, provider=LocalProvider( - channel=LocalChannel(), init_blocks=1, max_blocks=1, launcher=SingleNodeLauncher(), diff --git a/parsl/tests/scaling_tests/htex_local.py b/parsl/tests/scaling_tests/htex_local.py index f2bcf86963..f16f46af23 100644 --- a/parsl/tests/scaling_tests/htex_local.py +++ b/parsl/tests/scaling_tests/htex_local.py @@ -1,4 +1,3 @@ -from parsl.channels import LocalChannel from parsl.config import Config from parsl.executors import HighThroughputExecutor from parsl.providers import LocalProvider @@ -12,7 +11,6 @@ max_workers_per_node=8, encrypted=True, provider=LocalProvider( - channel=LocalChannel(), init_blocks=1, max_blocks=1, ), diff --git a/parsl/tests/sites/test_affinity.py b/parsl/tests/sites/test_affinity.py index 50d08ce830..792d490e03 100644 --- a/parsl/tests/sites/test_affinity.py +++ b/parsl/tests/sites/test_affinity.py @@ -5,7 +5,6 @@ import pytest from parsl import python_app -from parsl.channels import LocalChannel from parsl.config import Config from parsl.executors import HighThroughputExecutor from parsl.providers import LocalProvider @@ -22,7 +21,6 @@ def local_config(): available_accelerators=2, encrypted=True, provider=LocalProvider( - channel=LocalChannel(), init_blocks=1, max_blocks=1, ), diff --git a/parsl/tests/sites/test_worker_info.py b/parsl/tests/sites/test_worker_info.py index 4d9865da84..e55064a507 100644 --- a/parsl/tests/sites/test_worker_info.py +++ b/parsl/tests/sites/test_worker_info.py @@ -3,7 +3,6 @@ import pytest from parsl import python_app -from parsl.channels import LocalChannel from parsl.config import Config from parsl.executors import HighThroughputExecutor from parsl.providers import LocalProvider @@ -18,7 +17,6 @@ def local_config(): max_workers_per_node=4, encrypted=True, provider=LocalProvider( - channel=LocalChannel(), init_blocks=1, max_blocks=1, ), diff --git a/parsl/tests/test_htex/test_drain.py b/parsl/tests/test_htex/test_drain.py index efd0405863..663978f4c8 100644 --- a/parsl/tests/test_htex/test_drain.py +++ b/parsl/tests/test_htex/test_drain.py @@ -3,7 +3,6 @@ import pytest import parsl -from parsl.channels import LocalChannel from parsl.config import Config from parsl.executors import HighThroughputExecutor from parsl.launchers import SimpleLauncher @@ -28,7 +27,6 @@ def local_config(): cores_per_worker=1, encrypted=True, provider=LocalProvider( - channel=LocalChannel(), init_blocks=1, min_blocks=0, max_blocks=0, diff --git a/parsl/tests/test_htex/test_manager_selector_by_block.py b/parsl/tests/test_htex/test_manager_selector_by_block.py index 0933b581ff..1b2a4ee1f7 100644 --- a/parsl/tests/test_htex/test_manager_selector_by_block.py +++ b/parsl/tests/test_htex/test_manager_selector_by_block.py @@ -4,7 +4,6 @@ import parsl from parsl.app.app import bash_app, python_app -from parsl.channels import LocalChannel from parsl.config import Config from parsl.executors import HighThroughputExecutor from parsl.executors.high_throughput.manager_selector import ( @@ -31,7 +30,6 @@ def test_block_id_selection(try_assert): max_workers_per_node=1, manager_selector=BlockIdManagerSelector(), provider=LocalProvider( - channel=LocalChannel(), init_blocks=BLOCK_COUNT, max_blocks=BLOCK_COUNT, min_blocks=BLOCK_COUNT, diff --git a/parsl/tests/test_monitoring/test_htex_init_blocks_vs_monitoring.py b/parsl/tests/test_monitoring/test_htex_init_blocks_vs_monitoring.py index ada972e747..fabb172842 100644 --- a/parsl/tests/test_monitoring/test_htex_init_blocks_vs_monitoring.py +++ b/parsl/tests/test_monitoring/test_htex_init_blocks_vs_monitoring.py @@ -5,7 +5,6 @@ import pytest import parsl -from parsl.channels import LocalChannel from parsl.config import Config from parsl.executors import HighThroughputExecutor from parsl.launchers import SimpleLauncher @@ -22,7 +21,6 @@ def fresh_config(run_dir, strategy, db_url): cores_per_worker=1, encrypted=True, provider=LocalProvider( - channel=LocalChannel(), init_blocks=1, # min and max are set to 0 to ensure that we don't get # a block from ongoing strategy scaling, only from diff --git a/parsl/tests/test_providers/test_local_provider.py b/parsl/tests/test_providers/test_local_provider.py index 8d56d89ddf..c750814463 100644 --- a/parsl/tests/test_providers/test_local_provider.py +++ b/parsl/tests/test_providers/test_local_provider.py @@ -11,7 +11,6 @@ import pytest -from parsl.channels import LocalChannel from parsl.jobs.states import JobState from parsl.launchers import SingleNodeLauncher from parsl.providers import LocalProvider @@ -63,7 +62,7 @@ def _run_tests(p: LocalProvider): def test_local_channel(): with tempfile.TemporaryDirectory() as script_dir: script_dir = tempfile.mkdtemp() - p = LocalProvider(channel=LocalChannel(), launcher=SingleNodeLauncher(debug=False)) + p = LocalProvider(launcher=SingleNodeLauncher(debug=False)) p.script_dir = script_dir _run_tests(p) diff --git a/parsl/tests/test_scaling/test_regression_1621.py b/parsl/tests/test_scaling/test_regression_1621.py index 5aec750068..ea7d8f2631 100644 --- a/parsl/tests/test_scaling/test_regression_1621.py +++ b/parsl/tests/test_scaling/test_regression_1621.py @@ -3,7 +3,6 @@ import pytest import parsl -from parsl.channels import LocalChannel from parsl.config import Config from parsl.executors import HighThroughputExecutor from parsl.launchers import SimpleLauncher @@ -43,7 +42,6 @@ def test_one_block(tmpd_cwd): one app is invoked. this is a regression test. """ oneshot_provider = OneShotLocalProvider( - channel=LocalChannel(), init_blocks=0, min_blocks=0, max_blocks=10, diff --git a/parsl/tests/test_scaling/test_scale_down.py b/parsl/tests/test_scaling/test_scale_down.py index 7fb72ba507..79314d5173 100644 --- a/parsl/tests/test_scaling/test_scale_down.py +++ b/parsl/tests/test_scaling/test_scale_down.py @@ -29,7 +29,6 @@ def local_config(): max_workers_per_node=1, encrypted=True, provider=LocalProvider( - channel=LocalChannel(), init_blocks=0, max_blocks=_max_blocks, min_blocks=_min_blocks, diff --git a/parsl/tests/test_scaling/test_scale_down_htex_auto_scale.py b/parsl/tests/test_scaling/test_scale_down_htex_auto_scale.py index 016a51dc48..831bdf82af 100644 --- a/parsl/tests/test_scaling/test_scale_down_htex_auto_scale.py +++ b/parsl/tests/test_scaling/test_scale_down_htex_auto_scale.py @@ -4,7 +4,6 @@ import parsl from parsl import File, python_app -from parsl.channels import LocalChannel from parsl.config import Config from parsl.executors import HighThroughputExecutor from parsl.launchers import SingleNodeLauncher @@ -26,7 +25,6 @@ def local_config(): max_workers_per_node=1, encrypted=True, provider=LocalProvider( - channel=LocalChannel(), init_blocks=0, max_blocks=_max_blocks, min_blocks=_min_blocks, diff --git a/parsl/tests/test_scaling/test_scale_down_htex_unregistered.py b/parsl/tests/test_scaling/test_scale_down_htex_unregistered.py index 529877eac7..90a9b9ff1b 100644 --- a/parsl/tests/test_scaling/test_scale_down_htex_unregistered.py +++ b/parsl/tests/test_scaling/test_scale_down_htex_unregistered.py @@ -5,7 +5,6 @@ import parsl from parsl import File, python_app -from parsl.channels import LocalChannel from parsl.config import Config from parsl.executors import HighThroughputExecutor from parsl.jobs.states import TERMINAL_STATES, JobState @@ -31,7 +30,6 @@ def local_config(): encrypted=True, launch_cmd="sleep inf", provider=LocalProvider( - channel=LocalChannel(), init_blocks=1, max_blocks=_max_blocks, min_blocks=_min_blocks, diff --git a/parsl/tests/test_scaling/test_shutdown_scalein.py b/parsl/tests/test_scaling/test_shutdown_scalein.py index 1d1557ebb1..2505c79aca 100644 --- a/parsl/tests/test_scaling/test_shutdown_scalein.py +++ b/parsl/tests/test_scaling/test_shutdown_scalein.py @@ -4,7 +4,6 @@ import pytest import parsl -from parsl.channels import LocalChannel from parsl.config import Config from parsl.executors import HighThroughputExecutor from parsl.launchers import SimpleLauncher @@ -47,7 +46,6 @@ def test_shutdown_scalein_blocks(tmpd_cwd, try_assert): scaled in at DFK shutdown. """ accumulating_provider = AccumulatingLocalProvider( - channel=LocalChannel(), init_blocks=BLOCK_COUNT, min_blocks=0, max_blocks=0, From f6fd65f751d03e21bba9ec717c15bc06bf76d703 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Wed, 23 Oct 2024 14:47:10 +0000 Subject: [PATCH 11/71] Remove channels from example configs --- parsl/configs/cc_in2p3.py | 2 -- parsl/configs/frontera.py | 2 -- parsl/configs/htex_local.py | 2 -- 3 files changed, 6 deletions(-) diff --git a/parsl/configs/cc_in2p3.py b/parsl/configs/cc_in2p3.py index 631d76f9f5..f0140da55a 100644 --- a/parsl/configs/cc_in2p3.py +++ b/parsl/configs/cc_in2p3.py @@ -1,4 +1,3 @@ -from parsl.channels import LocalChannel from parsl.config import Config from parsl.executors import HighThroughputExecutor from parsl.providers import GridEngineProvider @@ -10,7 +9,6 @@ label='cc_in2p3_htex', max_workers_per_node=2, provider=GridEngineProvider( - channel=LocalChannel(), nodes_per_block=1, init_blocks=2, max_blocks=2, diff --git a/parsl/configs/frontera.py b/parsl/configs/frontera.py index a7b6f27b6c..25682bbe86 100644 --- a/parsl/configs/frontera.py +++ b/parsl/configs/frontera.py @@ -1,4 +1,3 @@ -from parsl.channels import LocalChannel from parsl.config import Config from parsl.executors import HighThroughputExecutor from parsl.launchers import SrunLauncher @@ -15,7 +14,6 @@ max_workers_per_node=1, # Set number of workers per node provider=SlurmProvider( cmd_timeout=60, # Add extra time for slow scheduler responses - channel=LocalChannel(), nodes_per_block=2, init_blocks=1, min_blocks=1, diff --git a/parsl/configs/htex_local.py b/parsl/configs/htex_local.py index 721dea767e..57549a4728 100644 --- a/parsl/configs/htex_local.py +++ b/parsl/configs/htex_local.py @@ -1,4 +1,3 @@ -from parsl.channels import LocalChannel from parsl.config import Config from parsl.executors import HighThroughputExecutor from parsl.providers import LocalProvider @@ -10,7 +9,6 @@ label="htex_local", cores_per_worker=1, provider=LocalProvider( - channel=LocalChannel(), init_blocks=1, max_blocks=1, ), From 23e47ef538f3ffc2e1a5a51b2ddc4598b6165926 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Wed, 23 Oct 2024 15:06:32 +0000 Subject: [PATCH 12/71] remove multichannelled-provider support - since adhoc provider was removed, this is not needed --- parsl/dataflow/dflow.py | 21 +++++---------------- parsl/providers/base.py | 8 -------- 2 files changed, 5 insertions(+), 24 deletions(-) diff --git a/parsl/dataflow/dflow.py b/parsl/dataflow/dflow.py index 344173c4b1..2a180f5238 100644 --- a/parsl/dataflow/dflow.py +++ b/parsl/dataflow/dflow.py @@ -1186,12 +1186,7 @@ def add_executors(self, executors: Sequence[ParslExecutor]) -> None: executor.provider.script_dir = os.path.join(self.run_dir, 'submit_scripts') os.makedirs(executor.provider.script_dir, exist_ok=True) - if hasattr(executor.provider, 'channels'): - logger.debug("Creating script_dir across multiple channels") - for channel in executor.provider.channels: - self._create_remote_dirs_over_channel(executor.provider, channel) - else: - self._create_remote_dirs_over_channel(executor.provider, executor.provider.channel) + self._create_remote_dirs_over_channel(executor.provider, executor.provider.channel) self.executors[executor.label] = executor executor.start() @@ -1277,16 +1272,10 @@ def cleanup(self) -> None: if hasattr(executor.provider, 'script_dir'): logger.info(f"Closing channel(s) for {executor.label}") - if hasattr(executor.provider, 'channels'): - for channel in executor.provider.channels: - logger.info(f"Closing channel {channel}") - channel.close() - logger.info(f"Closed channel {channel}") - else: - assert hasattr(executor.provider, 'channel'), "If provider has no .channels, it must have .channel" - logger.info(f"Closing channel {executor.provider.channel}") - executor.provider.channel.close() - logger.info(f"Closed channel {executor.provider.channel}") + assert hasattr(executor.provider, 'channel'), "If provider has no .channels, it must have .channel" + logger.info(f"Closing channel {executor.provider.channel}") + executor.provider.channel.close() + logger.info(f"Closed channel {executor.provider.channel}") logger.info(f"Closed executor channel(s) for {executor.label}") diff --git a/parsl/providers/base.py b/parsl/providers/base.py index 5475f86d4c..7611e7fdc3 100644 --- a/parsl/providers/base.py +++ b/parsl/providers/base.py @@ -161,11 +161,3 @@ class Channeled(): def __init__(self) -> None: self.channel: Channel pass - - -class MultiChanneled(): - """A marker type to indicate that parsl should manage multiple Channels for this provider""" - - def __init__(self) -> None: - self.channels: List[Channel] - pass From e2f0d19a07a7bcf3ce7881e22dbf84ef67ea9884 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Wed, 23 Oct 2024 15:15:57 +0000 Subject: [PATCH 13/71] Remove channel closing that is now a no-op --- parsl/channels/base.py | 6 ------ parsl/channels/local/local.py | 5 ----- parsl/dataflow/dflow.py | 11 ----------- 3 files changed, 22 deletions(-) diff --git a/parsl/channels/base.py b/parsl/channels/base.py index f650e7df3a..bba6e32bc2 100644 --- a/parsl/channels/base.py +++ b/parsl/channels/base.py @@ -71,12 +71,6 @@ def pull_file(self, remote_source: str, local_dir: str) -> str: ''' pass - @abstractmethod - def close(self) -> None: - ''' Closes the channel. - ''' - pass - @abstractmethod def makedirs(self, path: str, mode: int = 0o511, exist_ok: bool = False) -> None: """Create a directory. diff --git a/parsl/channels/local/local.py b/parsl/channels/local/local.py index b94629095e..cf09d2e49b 100644 --- a/parsl/channels/local/local.py +++ b/parsl/channels/local/local.py @@ -112,11 +112,6 @@ def push_file(self, source, dest_dir): def pull_file(self, remote_source, local_dir): return self.push_file(remote_source, local_dir) - def close(self) -> None: - ''' There's nothing to close here, and so this doesn't do anything - ''' - pass - def isdir(self, path): """Return true if the path refers to an existing directory. diff --git a/parsl/dataflow/dflow.py b/parsl/dataflow/dflow.py index 2a180f5238..ff5c4ff472 100644 --- a/parsl/dataflow/dflow.py +++ b/parsl/dataflow/dflow.py @@ -1268,17 +1268,6 @@ def cleanup(self) -> None: executor.shutdown() logger.info(f"Shut down executor {executor.label}") - if hasattr(executor, 'provider'): - if hasattr(executor.provider, 'script_dir'): - logger.info(f"Closing channel(s) for {executor.label}") - - assert hasattr(executor.provider, 'channel'), "If provider has no .channels, it must have .channel" - logger.info(f"Closing channel {executor.provider.channel}") - executor.provider.channel.close() - logger.info(f"Closed channel {executor.provider.channel}") - - logger.info(f"Closed executor channel(s) for {executor.label}") - logger.info("Terminated executors") self.time_completed = datetime.datetime.now() From 5142a97880f303c2b6bab374b0b88affbc520819 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Wed, 23 Oct 2024 15:19:07 +0000 Subject: [PATCH 14/71] Remove some LocalChannel imports that were not used --- .../test_scaling/test_regression_3568_scaledown_vs_MISSING.py | 1 - parsl/tests/test_scaling/test_scale_down.py | 1 - parsl/tests/test_staging/test_zip_in.py | 1 - parsl/tests/test_staging/test_zip_out.py | 1 - parsl/tests/test_staging/test_zip_to_zip.py | 1 - 5 files changed, 5 deletions(-) diff --git a/parsl/tests/test_scaling/test_regression_3568_scaledown_vs_MISSING.py b/parsl/tests/test_scaling/test_regression_3568_scaledown_vs_MISSING.py index a56b53af10..0c4a474b19 100644 --- a/parsl/tests/test_scaling/test_regression_3568_scaledown_vs_MISSING.py +++ b/parsl/tests/test_scaling/test_regression_3568_scaledown_vs_MISSING.py @@ -3,7 +3,6 @@ import pytest import parsl -from parsl.channels import LocalChannel from parsl.config import Config from parsl.executors import HighThroughputExecutor from parsl.launchers import WrappedLauncher diff --git a/parsl/tests/test_scaling/test_scale_down.py b/parsl/tests/test_scaling/test_scale_down.py index 79314d5173..a53630374f 100644 --- a/parsl/tests/test_scaling/test_scale_down.py +++ b/parsl/tests/test_scaling/test_scale_down.py @@ -5,7 +5,6 @@ import parsl from parsl import File, python_app -from parsl.channels import LocalChannel from parsl.config import Config from parsl.executors import HighThroughputExecutor from parsl.launchers import SingleNodeLauncher diff --git a/parsl/tests/test_staging/test_zip_in.py b/parsl/tests/test_staging/test_zip_in.py index 1f74f7e11b..9d43a4ab49 100644 --- a/parsl/tests/test_staging/test_zip_in.py +++ b/parsl/tests/test_staging/test_zip_in.py @@ -4,7 +4,6 @@ import pytest import parsl -from parsl.channels import LocalChannel from parsl.config import Config from parsl.data_provider.files import File from parsl.data_provider.zip import ZipAuthorityError, ZipFileStaging diff --git a/parsl/tests/test_staging/test_zip_out.py b/parsl/tests/test_staging/test_zip_out.py index e369031033..79fbb504d5 100644 --- a/parsl/tests/test_staging/test_zip_out.py +++ b/parsl/tests/test_staging/test_zip_out.py @@ -3,7 +3,6 @@ import pytest import parsl -from parsl.channels import LocalChannel from parsl.config import Config from parsl.data_provider.data_manager import default_staging from parsl.data_provider.files import File diff --git a/parsl/tests/test_staging/test_zip_to_zip.py b/parsl/tests/test_staging/test_zip_to_zip.py index 2c78e3bec2..3fea42167c 100644 --- a/parsl/tests/test_staging/test_zip_to_zip.py +++ b/parsl/tests/test_staging/test_zip_to_zip.py @@ -4,7 +4,6 @@ import pytest import parsl -from parsl.channels import LocalChannel from parsl.config import Config from parsl.data_provider.files import File from parsl.data_provider.zip import ZipAuthorityError, ZipFileStaging From 3d79eecb686fff52ee369f50f7c5d0297dc6d217 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Wed, 23 Oct 2024 15:24:11 +0000 Subject: [PATCH 15/71] Remove unused BadPermsScriptPath -- not needed after removing SSH channels --- docs/reference.rst | 2 -- parsl/channels/errors.py | 26 -------------------------- 2 files changed, 28 deletions(-) diff --git a/docs/reference.rst b/docs/reference.rst index 60dc6733f5..5fffc78b1a 100644 --- a/docs/reference.rst +++ b/docs/reference.rst @@ -171,8 +171,6 @@ Exceptions parsl.providers.errors.SchedulerMissingArgs parsl.providers.errors.ScriptPathError parsl.channels.errors.ChannelError - parsl.channels.errors.BadScriptPath - parsl.channels.errors.BadPermsScriptPath parsl.channels.errors.FileExists parsl.channels.errors.AuthException parsl.channels.errors.FileCopyException diff --git a/parsl/channels/errors.py b/parsl/channels/errors.py index 9a6b19c134..1c9bf65c6d 100644 --- a/parsl/channels/errors.py +++ b/parsl/channels/errors.py @@ -19,32 +19,6 @@ def __str__(self) -> str: return "Hostname:{0}, Reason:{1}".format(self.hostname, self.reason) -class BadScriptPath(ChannelError): - ''' An error raised during execution of an app. - What this exception contains depends entirely on context - Contains: - reason(string) - e (paramiko exception object) - hostname (string) - ''' - - def __init__(self, e: Exception, hostname: str) -> None: - super().__init__("Inaccessible remote script dir. Specify script_dir", e, hostname) - - -class BadPermsScriptPath(ChannelError): - ''' User does not have permissions to access the script_dir on the remote site - - Contains: - reason(string) - e (paramiko exception object) - hostname (string) - ''' - - def __init__(self, e: Exception, hostname: str) -> None: - super().__init__("User does not have permissions to access the script_dir", e, hostname) - - class FileExists(ChannelError): ''' Push or pull of file over channel fails since a file of the name already exists on the destination. From 0983a73622d4ae9d8437f8201138558b300632b5 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Wed, 23 Oct 2024 15:30:39 +0000 Subject: [PATCH 16/71] apply PR #3651 - remove file exists --- docs/reference.rst | 1 - parsl/channels/errors.py | 17 ----------------- 2 files changed, 18 deletions(-) diff --git a/docs/reference.rst b/docs/reference.rst index 5fffc78b1a..82cd94698f 100644 --- a/docs/reference.rst +++ b/docs/reference.rst @@ -171,7 +171,6 @@ Exceptions parsl.providers.errors.SchedulerMissingArgs parsl.providers.errors.ScriptPathError parsl.channels.errors.ChannelError - parsl.channels.errors.FileExists parsl.channels.errors.AuthException parsl.channels.errors.FileCopyException parsl.executors.high_throughput.errors.WorkerLost diff --git a/parsl/channels/errors.py b/parsl/channels/errors.py index 1c9bf65c6d..981c8ffcc1 100644 --- a/parsl/channels/errors.py +++ b/parsl/channels/errors.py @@ -1,7 +1,5 @@ ''' Exceptions raise by Apps. ''' -from typing import Optional - from parsl.errors import ParslError @@ -19,21 +17,6 @@ def __str__(self) -> str: return "Hostname:{0}, Reason:{1}".format(self.hostname, self.reason) -class FileExists(ChannelError): - ''' Push or pull of file over channel fails since a file of the name already - exists on the destination. - - Contains: - reason(string) - e (paramiko exception object) - hostname (string) - ''' - - def __init__(self, e: Exception, hostname: str, filename: Optional[str] = None) -> None: - super().__init__("File name collision in channel transport phase: {}".format(filename), - e, hostname) - - class AuthException(ChannelError): ''' An error raised during execution of an app. What this exception contains depends entirely on context From 25f7584415dbdb0ddded47ab50bb1a2bc953b3ac Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Wed, 23 Oct 2024 15:32:11 +0000 Subject: [PATCH 17/71] Remove AuthException unneeded since removal of ssh channels --- docs/reference.rst | 1 - parsl/channels/errors.py | 13 ------------- 2 files changed, 14 deletions(-) diff --git a/docs/reference.rst b/docs/reference.rst index 82cd94698f..460e524193 100644 --- a/docs/reference.rst +++ b/docs/reference.rst @@ -171,7 +171,6 @@ Exceptions parsl.providers.errors.SchedulerMissingArgs parsl.providers.errors.ScriptPathError parsl.channels.errors.ChannelError - parsl.channels.errors.AuthException parsl.channels.errors.FileCopyException parsl.executors.high_throughput.errors.WorkerLost parsl.executors.high_throughput.interchange.ManagerLost diff --git a/parsl/channels/errors.py b/parsl/channels/errors.py index 981c8ffcc1..effaea9548 100644 --- a/parsl/channels/errors.py +++ b/parsl/channels/errors.py @@ -17,19 +17,6 @@ def __str__(self) -> str: return "Hostname:{0}, Reason:{1}".format(self.hostname, self.reason) -class AuthException(ChannelError): - ''' An error raised during execution of an app. - What this exception contains depends entirely on context - Contains: - reason(string) - e (paramiko exception object) - hostname (string) - ''' - - def __init__(self, e: Exception, hostname: str) -> None: - super().__init__("Authentication to remote server failed", e, hostname) - - class FileCopyException(ChannelError): ''' File copy operation failed From 26c46b981fae72dabb99f29f3812d3c5171ff194 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Wed, 23 Oct 2024 15:34:23 +0000 Subject: [PATCH 18/71] reparent FileCopyException and remove hostname which is only ever localhost now --- docs/reference.rst | 1 - parsl/channels/errors.py | 24 ++++-------------------- parsl/channels/local/local.py | 2 +- 3 files changed, 5 insertions(+), 22 deletions(-) diff --git a/docs/reference.rst b/docs/reference.rst index 460e524193..fa3c8423de 100644 --- a/docs/reference.rst +++ b/docs/reference.rst @@ -170,7 +170,6 @@ Exceptions parsl.providers.errors.ScaleOutFailed parsl.providers.errors.SchedulerMissingArgs parsl.providers.errors.ScriptPathError - parsl.channels.errors.ChannelError parsl.channels.errors.FileCopyException parsl.executors.high_throughput.errors.WorkerLost parsl.executors.high_throughput.interchange.ManagerLost diff --git a/parsl/channels/errors.py b/parsl/channels/errors.py index effaea9548..82107b387e 100644 --- a/parsl/channels/errors.py +++ b/parsl/channels/errors.py @@ -3,28 +3,12 @@ from parsl.errors import ParslError -class ChannelError(ParslError): - """ Base class for all exceptions - - Only to be invoked when only a more specific error is not available. - """ - def __init__(self, reason: str, e: Exception, hostname: str) -> None: - self.reason = reason - self.e = e - self.hostname = hostname - - def __str__(self) -> str: - return "Hostname:{0}, Reason:{1}".format(self.hostname, self.reason) - - -class FileCopyException(ChannelError): +class FileCopyException(ParslError): ''' File copy operation failed Contains: - reason(string) - e (paramiko exception object) - hostname (string) + e (parent exception object) ''' - def __init__(self, e: Exception, hostname: str) -> None: - super().__init__("File copy failed due to {0}".format(e), e, hostname) + def __init__(self, e: Exception) -> None: + super().__init__("File copy failed due to {0}".format(e)) diff --git a/parsl/channels/local/local.py b/parsl/channels/local/local.py index cf09d2e49b..d82133155f 100644 --- a/parsl/channels/local/local.py +++ b/parsl/channels/local/local.py @@ -102,7 +102,7 @@ def push_file(self, source, dest_dir): os.chmod(local_dest, 0o700) except OSError as e: - raise FileCopyException(e, self.hostname) + raise FileCopyException(e) else: os.chmod(local_dest, 0o700) From baeb29478bd146700549ce402556bcf1c3a9a312 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Wed, 23 Oct 2024 15:35:52 +0000 Subject: [PATCH 19/71] Remove LocalChannel hostname attribute, always localhost and now unused --- parsl/channels/local/local.py | 1 - 1 file changed, 1 deletion(-) diff --git a/parsl/channels/local/local.py b/parsl/channels/local/local.py index d82133155f..8eaa8d1b65 100644 --- a/parsl/channels/local/local.py +++ b/parsl/channels/local/local.py @@ -25,7 +25,6 @@ def __init__(self, userhome=".", envs={}, script_dir=None): - script_dir (string): Directory to place scripts ''' self.userhome = os.path.abspath(userhome) - self.hostname = "localhost" self.envs = envs local_env = os.environ.copy() self._envs = copy.deepcopy(local_env) From 048d5572307d062e03bfe9a63e76d01598a2f797 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Wed, 23 Oct 2024 15:47:00 +0000 Subject: [PATCH 20/71] Remove remote-fs detector and behaviour --- parsl/dataflow/dflow.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/parsl/dataflow/dflow.py b/parsl/dataflow/dflow.py index ff5c4ff472..f948258275 100644 --- a/parsl/dataflow/dflow.py +++ b/parsl/dataflow/dflow.py @@ -6,7 +6,6 @@ import inspect import logging import os -import pathlib import pickle import random import sys @@ -1164,13 +1163,6 @@ def _create_remote_dirs_over_channel(self, provider: ExecutionProvider, channel: channel.script_dir = os.path.join(run_dir, 'submit_scripts') # type: ignore[unreachable] - # Only create dirs if we aren't on a shared-fs - if not channel.isdir(run_dir): - parent, child = pathlib.Path(run_dir).parts[-2:] - remote_run_dir = os.path.join(parent, child) - channel.script_dir = os.path.join(remote_run_dir, 'remote_submit_scripts') - provider.script_dir = os.path.join(run_dir, 'local_submit_scripts') - channel.makedirs(channel.script_dir, exist_ok=True) def add_executors(self, executors: Sequence[ParslExecutor]) -> None: From a8c64f4d51d7b9b2da63b25e34ff9a49dcdc4da1 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Wed, 23 Oct 2024 15:48:49 +0000 Subject: [PATCH 21/71] Remove channel.isdir, no longer needed since directly previous commit --- parsl/channels/base.py | 11 ----------- parsl/channels/local/local.py | 11 ----------- 2 files changed, 22 deletions(-) diff --git a/parsl/channels/base.py b/parsl/channels/base.py index bba6e32bc2..be28d2b1d1 100644 --- a/parsl/channels/base.py +++ b/parsl/channels/base.py @@ -88,17 +88,6 @@ def makedirs(self, path: str, mode: int = 0o511, exist_ok: bool = False) -> None """ pass - @abstractmethod - def isdir(self, path: str) -> bool: - """Return true if the path refers to an existing directory. - - Parameters - ---------- - path : str - Path of directory to check. - """ - pass - @abstractmethod def abspath(self, path: str) -> str: """Return the absolute path. diff --git a/parsl/channels/local/local.py b/parsl/channels/local/local.py index 8eaa8d1b65..64df45f3fd 100644 --- a/parsl/channels/local/local.py +++ b/parsl/channels/local/local.py @@ -111,17 +111,6 @@ def push_file(self, source, dest_dir): def pull_file(self, remote_source, local_dir): return self.push_file(remote_source, local_dir) - def isdir(self, path): - """Return true if the path refers to an existing directory. - - Parameters - ---------- - path : str - Path of directory to check. - """ - - return os.path.isdir(path) - def makedirs(self, path, mode=0o700, exist_ok=False): """Create a directory. From 3f761b4931ea061d0d4d4928b5a20ab059142ba0 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Wed, 23 Oct 2024 15:53:16 +0000 Subject: [PATCH 22/71] Remove Channels.abspath that is unused --- parsl/channels/base.py | 11 ----------- parsl/channels/local/local.py | 12 +----------- parsl/channels/ssh/ssh.py | 10 ---------- 3 files changed, 1 insertion(+), 32 deletions(-) diff --git a/parsl/channels/base.py b/parsl/channels/base.py index 18a573d0e6..754e89c385 100644 --- a/parsl/channels/base.py +++ b/parsl/channels/base.py @@ -120,14 +120,3 @@ def isdir(self, path: str) -> bool: Path of directory to check. """ pass - - @abstractmethod - def abspath(self, path: str) -> str: - """Return the absolute path. - - Parameters - ---------- - path : str - Path for which the absolute path will be returned. - """ - pass diff --git a/parsl/channels/local/local.py b/parsl/channels/local/local.py index b94629095e..8a52b853d3 100644 --- a/parsl/channels/local/local.py +++ b/parsl/channels/local/local.py @@ -145,16 +145,6 @@ def makedirs(self, path, mode=0o700, exist_ok=False): return os.makedirs(path, mode, exist_ok) - def abspath(self, path): - """Return the absolute path. - - Parameters - ---------- - path : str - Path for which the absolute path will be returned. - """ - return os.path.abspath(path) - @property def script_dir(self): return self._script_dir @@ -162,5 +152,5 @@ def script_dir(self): @script_dir.setter def script_dir(self, value): if value is not None: - value = self.abspath(value) + value = os.path.abspath(value) self._script_dir = value diff --git a/parsl/channels/ssh/ssh.py b/parsl/channels/ssh/ssh.py index c53a26b831..0bcb8a06b4 100644 --- a/parsl/channels/ssh/ssh.py +++ b/parsl/channels/ssh/ssh.py @@ -287,16 +287,6 @@ def makedirs(self, path, mode=0o700, exist_ok=False): self.execute_wait('mkdir -p {}'.format(path)) self._valid_sftp_client().chmod(path, mode) - def abspath(self, path): - """Return the absolute path on the remote side. - - Parameters - ---------- - path : str - Path for which the absolute path will be returned. - """ - return self._valid_sftp_client().normalize(path) - @property def script_dir(self): return self._script_dir From fd920481b8f9364f4e0d206052064b976f66ad16 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 08:33:31 +0000 Subject: [PATCH 23/71] Remove un-used Channeled interface --- parsl/providers/base.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/parsl/providers/base.py b/parsl/providers/base.py index 7611e7fdc3..5daf3b0d1c 100644 --- a/parsl/providers/base.py +++ b/parsl/providers/base.py @@ -154,10 +154,3 @@ def status_polling_interval(self) -> int: :return: the number of seconds to wait between calls to status() """ pass - - -class Channeled(): - """A marker type to indicate that parsl should manage a Channel for this provider""" - def __init__(self) -> None: - self.channel: Channel - pass From 83bc7ef3d209254b12b5ad38b3c7da1b5d446219 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 08:45:14 +0000 Subject: [PATCH 24/71] remove test for channel closing that was removed in e2f0d19a07a7bcf3ce7881e22dbf84ef67ea9884 - fails tests otherwise --- parsl/tests/test_channels/test_dfk_close.py | 26 --------------------- 1 file changed, 26 deletions(-) delete mode 100644 parsl/tests/test_channels/test_dfk_close.py diff --git a/parsl/tests/test_channels/test_dfk_close.py b/parsl/tests/test_channels/test_dfk_close.py deleted file mode 100644 index 05b2e9395f..0000000000 --- a/parsl/tests/test_channels/test_dfk_close.py +++ /dev/null @@ -1,26 +0,0 @@ -from unittest.mock import Mock - -import pytest - -import parsl -from parsl.channels.base import Channel -from parsl.executors import HighThroughputExecutor -from parsl.providers import LocalProvider - - -@pytest.mark.local -def test_dfk_close(): - - mock_channel = Mock(spec=Channel) - - # block settings all 0 because the mock channel won't be able to - # do anything to make a block exist - p = LocalProvider(channel=mock_channel, init_blocks=0, min_blocks=0, max_blocks=0) - - e = HighThroughputExecutor(provider=p) - - c = parsl.Config(executors=[e]) - with parsl.load(c): - pass - - assert mock_channel.close.called From 4bdd727699ca9b885473e7b86974d40636325ec3 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 08:47:46 +0000 Subject: [PATCH 25/71] remove unused import from 2 commits up --- parsl/providers/base.py | 1 - 1 file changed, 1 deletion(-) diff --git a/parsl/providers/base.py b/parsl/providers/base.py index 5daf3b0d1c..c8c17c6f5b 100644 --- a/parsl/providers/base.py +++ b/parsl/providers/base.py @@ -2,7 +2,6 @@ from abc import ABCMeta, abstractmethod, abstractproperty from typing import Any, Dict, List, Optional -from parsl.channels.base import Channel from parsl.jobs.states import JobStatus logger = logging.getLogger(__name__) From e1263ea255d33ed0961677900f60704001264b65 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 11:44:51 +0000 Subject: [PATCH 26/71] Remove parameters to LocalChannel, as part of move towards a hard-coded LocalChannel() rather than allowing users to specify a parameterised LocalChannel Two tests are amended so that they behave as if the DFK is setting script dir on them now (the behaviour when the removed script_dir parameter is not set) This is intended to be followed by more commits than can remove/rewrite code assuming these values are constants. --- parsl/channels/local/local.py | 7 ++++--- parsl/tests/test_providers/test_pbspro_template.py | 3 ++- parsl/tests/test_providers/test_slurm_template.py | 3 ++- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/parsl/channels/local/local.py b/parsl/channels/local/local.py index 3f5ee375c0..f93ca30e4b 100644 --- a/parsl/channels/local/local.py +++ b/parsl/channels/local/local.py @@ -16,7 +16,7 @@ class LocalChannel(Channel, RepresentationMixin): and done so infrequently that they do not need a persistent channel ''' - def __init__(self, userhome=".", envs={}, script_dir=None): + def __init__(self): ''' Initialize the local channel. script_dir is required by set to a default. KwArgs: @@ -24,12 +24,13 @@ def __init__(self, userhome=".", envs={}, script_dir=None): - envs (dict) : A dictionary of env variables to be set when launching the shell - script_dir (string): Directory to place scripts ''' - self.userhome = os.path.abspath(userhome) + self.userhome = os.path.abspath(".") + envs = {} self.envs = envs local_env = os.environ.copy() self._envs = copy.deepcopy(local_env) self._envs.update(envs) - self.script_dir = script_dir + self.script_dir = None def execute_wait(self, cmd, walltime=None, envs={}): ''' Synchronously execute a commandline string on the shell. diff --git a/parsl/tests/test_providers/test_pbspro_template.py b/parsl/tests/test_providers/test_pbspro_template.py index 1a61118047..dec987ccbb 100644 --- a/parsl/tests/test_providers/test_pbspro_template.py +++ b/parsl/tests/test_providers/test_pbspro_template.py @@ -12,9 +12,10 @@ def test_submit_script_basic(tmp_path): """Test slurm resources table""" provider = PBSProProvider( - queue="debug", channel=LocalChannel(script_dir=tmp_path) + queue="debug", channel=LocalChannel() ) provider.script_dir = tmp_path + provider.channel.script_dir = tmp_path job_id = str(random.randint(55000, 59000)) provider.execute_wait = mock.Mock(spec=PBSProProvider.execute_wait) provider.execute_wait.return_value = (0, job_id, "") diff --git a/parsl/tests/test_providers/test_slurm_template.py b/parsl/tests/test_providers/test_slurm_template.py index 72bacf4e1d..57fd8e4d0b 100644 --- a/parsl/tests/test_providers/test_slurm_template.py +++ b/parsl/tests/test_providers/test_slurm_template.py @@ -13,9 +13,10 @@ def test_submit_script_basic(tmp_path): """Test slurm resources table""" provider = SlurmProvider( - partition="debug", channel=LocalChannel(script_dir=tmp_path) + partition="debug", channel=LocalChannel() ) provider.script_dir = tmp_path + provider.channel.script_dir = tmp_path job_id = str(random.randint(55000, 59000)) provider.execute_wait = mock.MagicMock(spec=SlurmProvider.execute_wait) provider.execute_wait.return_value = (0, f"Submitted batch job {job_id}", "") From da067fe724fb30ee9f3db87420474cda1fad9884 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 11:55:25 +0000 Subject: [PATCH 27/71] Remove `if` which always takes the same path. Since e1263ea255d33ed0961677900f60704001264b65 script_dir is always None at creation time, so this if statement always takes the same path. (caution: if a channel is reused between runs, which is not allowed, the previous broken behaviour might be different broken behaviour now) --- parsl/dataflow/dflow.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/parsl/dataflow/dflow.py b/parsl/dataflow/dflow.py index f948258275..f03865eaeb 100644 --- a/parsl/dataflow/dflow.py +++ b/parsl/dataflow/dflow.py @@ -1153,15 +1153,8 @@ def _create_remote_dirs_over_channel(self, provider: ExecutionProvider, channel: Channel over which the remote dirs are to be created """ run_dir = self.run_dir - if channel.script_dir is None: - # This case will be detected as unreachable by mypy, because of - # the type of script_dir, which is str, not Optional[str]. - # The type system doesn't represent the initialized/uninitialized - # state of a channel so cannot represent that a channel needs - # its script directory set or not. - - channel.script_dir = os.path.join(run_dir, 'submit_scripts') # type: ignore[unreachable] + channel.script_dir = os.path.join(run_dir, 'submit_scripts') channel.makedirs(channel.script_dir, exist_ok=True) From 079b54e32932ff91cd3463632fe396f09b70ebae Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 11:57:48 +0000 Subject: [PATCH 28/71] Remove unused provider parameter This has been unused since removal of non-shared filesystem codepath here --- parsl/dataflow/dflow.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/parsl/dataflow/dflow.py b/parsl/dataflow/dflow.py index f03865eaeb..4d1933402b 100644 --- a/parsl/dataflow/dflow.py +++ b/parsl/dataflow/dflow.py @@ -48,7 +48,6 @@ from parsl.monitoring.message_type import MessageType from parsl.monitoring.remote import monitor_wrapper from parsl.process_loggers import wrap_with_logs -from parsl.providers.base import ExecutionProvider from parsl.usage_tracking.usage import UsageTracker from parsl.utils import Timer, get_all_checkpoints, get_std_fname_mode, get_version @@ -1142,13 +1141,11 @@ def log_task_states(self) -> None: logger.info("End of summary") - def _create_remote_dirs_over_channel(self, provider: ExecutionProvider, channel: Channel) -> None: + def _create_remote_dirs_over_channel(self, channel: Channel) -> None: """Create script directories across a channel Parameters ---------- - provider: Provider obj - Provider for which scripts dirs are being created channel: Channel obj Channel over which the remote dirs are to be created """ @@ -1171,7 +1168,7 @@ def add_executors(self, executors: Sequence[ParslExecutor]) -> None: executor.provider.script_dir = os.path.join(self.run_dir, 'submit_scripts') os.makedirs(executor.provider.script_dir, exist_ok=True) - self._create_remote_dirs_over_channel(executor.provider, executor.provider.channel) + self._create_remote_dirs_over_channel(executor.provider.channel) self.executors[executor.label] = executor executor.start() From 5a2bae35afe202274d5acec0cd4c554ca755ae01 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 12:02:44 +0000 Subject: [PATCH 29/71] Rename script dir creation method to what it actually does --- parsl/dataflow/dflow.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/parsl/dataflow/dflow.py b/parsl/dataflow/dflow.py index 4d1933402b..d377b32e9e 100644 --- a/parsl/dataflow/dflow.py +++ b/parsl/dataflow/dflow.py @@ -1141,7 +1141,7 @@ def log_task_states(self) -> None: logger.info("End of summary") - def _create_remote_dirs_over_channel(self, channel: Channel) -> None: + def _create_script_dir(self, channel: Channel) -> None: """Create script directories across a channel Parameters @@ -1168,7 +1168,7 @@ def add_executors(self, executors: Sequence[ParslExecutor]) -> None: executor.provider.script_dir = os.path.join(self.run_dir, 'submit_scripts') os.makedirs(executor.provider.script_dir, exist_ok=True) - self._create_remote_dirs_over_channel(executor.provider.channel) + self._create_script_dir(executor.provider.channel) self.executors[executor.label] = executor executor.start() From ae81901f9302c104a4de43787e063764d3050851 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 12:14:42 +0000 Subject: [PATCH 30/71] Remove user configurability of channels in providers --- parsl/providers/cluster_provider.py | 4 ++-- parsl/providers/cobalt/cobalt.py | 3 --- parsl/providers/condor/condor.py | 5 ----- parsl/providers/grid_engine/grid_engine.py | 3 --- parsl/providers/local/local.py | 3 +-- parsl/providers/lsf/lsf.py | 3 --- parsl/providers/pbspro/pbspro.py | 5 +---- parsl/providers/slurm/slurm.py | 4 ---- parsl/providers/torque/torque.py | 3 --- 9 files changed, 4 insertions(+), 29 deletions(-) diff --git a/parsl/providers/cluster_provider.py b/parsl/providers/cluster_provider.py index 27e0c57e88..ed74b97862 100644 --- a/parsl/providers/cluster_provider.py +++ b/parsl/providers/cluster_provider.py @@ -2,6 +2,7 @@ from abc import abstractmethod from string import Template +from parsl.channels.local.local import LocalChannel from parsl.launchers.base import Launcher from parsl.launchers.errors import BadLauncher from parsl.providers.base import ExecutionProvider @@ -42,7 +43,6 @@ class ClusterProvider(ExecutionProvider): def __init__(self, label, - channel, nodes_per_block, init_blocks, min_blocks, @@ -53,7 +53,7 @@ def __init__(self, cmd_timeout=10): self._label = label - self.channel = channel + self.channel = LocalChannel() self.nodes_per_block = nodes_per_block self.init_blocks = init_blocks self.min_blocks = min_blocks diff --git a/parsl/providers/cobalt/cobalt.py b/parsl/providers/cobalt/cobalt.py index 4cbdde90be..6a6c446da9 100644 --- a/parsl/providers/cobalt/cobalt.py +++ b/parsl/providers/cobalt/cobalt.py @@ -3,7 +3,6 @@ import time import warnings -from parsl.channels import LocalChannel from parsl.jobs.states import JobState, JobStatus from parsl.launchers import AprunLauncher from parsl.providers.cluster_provider import ClusterProvider @@ -55,7 +54,6 @@ class CobaltProvider(ClusterProvider, RepresentationMixin): :class:`~parsl.launchers.SingleNodeLauncher` """ def __init__(self, - channel=LocalChannel(), nodes_per_block=1, init_blocks=0, min_blocks=0, @@ -70,7 +68,6 @@ def __init__(self, cmd_timeout=10): label = 'cobalt' super().__init__(label, - channel=channel, nodes_per_block=nodes_per_block, init_blocks=init_blocks, min_blocks=min_blocks, diff --git a/parsl/providers/condor/condor.py b/parsl/providers/condor/condor.py index 15734ee871..063233f0ae 100644 --- a/parsl/providers/condor/condor.py +++ b/parsl/providers/condor/condor.py @@ -5,7 +5,6 @@ import typeguard -from parsl.channels import LocalChannel from parsl.jobs.states import JobState, JobStatus from parsl.launchers import SingleNodeLauncher from parsl.launchers.base import Launcher @@ -18,8 +17,6 @@ from typing import Dict, List, Optional -from parsl.channels.base import Channel - # See http://pages.cs.wisc.edu/~adesmet/status.html translate_table = { '1': JobState.PENDING, @@ -77,7 +74,6 @@ class CondorProvider(RepresentationMixin, ClusterProvider): """ @typeguard.typechecked def __init__(self, - channel: Channel = LocalChannel(), nodes_per_block: int = 1, cores_per_slot: Optional[int] = None, mem_per_slot: Optional[float] = None, @@ -98,7 +94,6 @@ def __init__(self, label = 'condor' super().__init__(label, - channel, nodes_per_block, init_blocks, min_blocks, diff --git a/parsl/providers/grid_engine/grid_engine.py b/parsl/providers/grid_engine/grid_engine.py index 0e68aaaea2..96bf5ee375 100644 --- a/parsl/providers/grid_engine/grid_engine.py +++ b/parsl/providers/grid_engine/grid_engine.py @@ -2,7 +2,6 @@ import os import time -from parsl.channels import LocalChannel from parsl.jobs.states import JobState, JobStatus from parsl.launchers import SingleNodeLauncher from parsl.providers.cluster_provider import ClusterProvider @@ -60,7 +59,6 @@ class GridEngineProvider(ClusterProvider, RepresentationMixin): """ def __init__(self, - channel=LocalChannel(), nodes_per_block=1, init_blocks=1, min_blocks=0, @@ -74,7 +72,6 @@ def __init__(self, queue=None): label = 'grid_engine' super().__init__(label, - channel, nodes_per_block, init_blocks, min_blocks, diff --git a/parsl/providers/local/local.py b/parsl/providers/local/local.py index f13521466a..221b97b532 100644 --- a/parsl/providers/local/local.py +++ b/parsl/providers/local/local.py @@ -40,7 +40,6 @@ class LocalProvider(ExecutionProvider, RepresentationMixin): """ def __init__(self, - channel=LocalChannel(), nodes_per_block=1, launcher=SingleNodeLauncher(), init_blocks=1, @@ -50,7 +49,7 @@ def __init__(self, cmd_timeout=30, parallelism=1, move_files=None): - self.channel = channel + self.channel = LocalChannel() self._label = 'local' self.nodes_per_block = nodes_per_block self.launcher = launcher diff --git a/parsl/providers/lsf/lsf.py b/parsl/providers/lsf/lsf.py index 2eaff23c3e..972fbd49b7 100644 --- a/parsl/providers/lsf/lsf.py +++ b/parsl/providers/lsf/lsf.py @@ -3,7 +3,6 @@ import os import time -from parsl.channels import LocalChannel from parsl.jobs.states import JobState, JobStatus from parsl.launchers import SingleNodeLauncher from parsl.providers.cluster_provider import ClusterProvider @@ -76,7 +75,6 @@ class LSFProvider(ClusterProvider, RepresentationMixin): """ def __init__(self, - channel=LocalChannel(), nodes_per_block=1, cores_per_block=None, cores_per_node=None, @@ -96,7 +94,6 @@ def __init__(self, launcher=SingleNodeLauncher()): label = 'LSF' super().__init__(label, - channel, nodes_per_block, init_blocks, min_blocks, diff --git a/parsl/providers/pbspro/pbspro.py b/parsl/providers/pbspro/pbspro.py index e7a1bb4a30..efaaf7ec01 100644 --- a/parsl/providers/pbspro/pbspro.py +++ b/parsl/providers/pbspro/pbspro.py @@ -3,7 +3,6 @@ import os import time -from parsl.channels import LocalChannel from parsl.jobs.states import JobState, JobStatus from parsl.launchers import SingleNodeLauncher from parsl.providers.pbspro.template import template_string @@ -49,7 +48,6 @@ class PBSProProvider(TorqueProvider): :class:`~parsl.launchers.SingleNodeLauncher`. """ def __init__(self, - channel=LocalChannel(), account=None, queue=None, scheduler_options='', @@ -64,8 +62,7 @@ def __init__(self, launcher=SingleNodeLauncher(), walltime="00:20:00", cmd_timeout=120): - super().__init__(channel, - account, + super().__init__(account, queue, scheduler_options, worker_init, diff --git a/parsl/providers/slurm/slurm.py b/parsl/providers/slurm/slurm.py index 43719f9da5..ad87260cdc 100644 --- a/parsl/providers/slurm/slurm.py +++ b/parsl/providers/slurm/slurm.py @@ -7,8 +7,6 @@ import typeguard -from parsl.channels import LocalChannel -from parsl.channels.base import Channel from parsl.jobs.states import JobState, JobStatus from parsl.launchers import SingleNodeLauncher from parsl.launchers.base import Launcher @@ -114,7 +112,6 @@ def __init__(self, account: Optional[str] = None, qos: Optional[str] = None, constraint: Optional[str] = None, - channel: Channel = LocalChannel(), nodes_per_block: int = 1, cores_per_node: Optional[int] = None, mem_per_node: Optional[int] = None, @@ -132,7 +129,6 @@ def __init__(self, launcher: Launcher = SingleNodeLauncher()): label = 'slurm' super().__init__(label, - channel, nodes_per_block, init_blocks, min_blocks, diff --git a/parsl/providers/torque/torque.py b/parsl/providers/torque/torque.py index aa030a60f6..44c64e1cc1 100644 --- a/parsl/providers/torque/torque.py +++ b/parsl/providers/torque/torque.py @@ -2,7 +2,6 @@ import os import time -from parsl.channels import LocalChannel from parsl.jobs.states import JobState, JobStatus from parsl.launchers import AprunLauncher from parsl.providers.cluster_provider import ClusterProvider @@ -63,7 +62,6 @@ class TorqueProvider(ClusterProvider, RepresentationMixin): """ def __init__(self, - channel=LocalChannel(), account=None, queue=None, scheduler_options='', @@ -78,7 +76,6 @@ def __init__(self, cmd_timeout=120): label = 'torque' super().__init__(label, - channel, nodes_per_block, init_blocks, min_blocks, From 67b7471f6cdacafc8e5ddd868f9c59d0c698a383 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 12:24:20 +0000 Subject: [PATCH 31/71] Remove accessors for script_dir, treating it as a normal attribute now The setter previously made this an absolute path. But the code that sets this in the DFK already provides an absolute path. --- parsl/channels/base.py | 27 +++++++-------------------- parsl/channels/local/local.py | 10 ---------- 2 files changed, 7 insertions(+), 30 deletions(-) diff --git a/parsl/channels/base.py b/parsl/channels/base.py index 6afae5a980..6316ad6287 100644 --- a/parsl/channels/base.py +++ b/parsl/channels/base.py @@ -1,4 +1,4 @@ -from abc import ABCMeta, abstractmethod, abstractproperty +from abc import ABCMeta, abstractmethod from typing import Dict, Tuple @@ -8,6 +8,12 @@ class Channel(metaclass=ABCMeta): LocalChannel. """ + # Script dir must be an absolute path + # This is initialized to None, which violates the type specification, + # and so code using script_dir must make sure it doesn't expect this + # to be not-None until after the DFK has initialized the channel + script_dir: str + @abstractmethod def execute_wait(self, cmd: str, walltime: int = 0, envs: Dict[str, str] = {}) -> Tuple[int, str, str]: ''' Executes the cmd, with a defined walltime. @@ -24,25 +30,6 @@ def execute_wait(self, cmd: str, walltime: int = 0, envs: Dict[str, str] = {}) - ''' pass - @abstractproperty - def script_dir(self) -> str: - ''' This is a property. Returns the directory assigned for storing all internal scripts such as - scheduler submit scripts. This is usually where error logs from the scheduler would reside on the - channel destination side. - - Args: - - None - - Returns: - - Channel script dir - ''' - pass - - # DFK expects to be able to modify this, so it needs to be in the abstract class - @script_dir.setter - def script_dir(self, value: str) -> None: - pass - @abstractmethod def push_file(self, source: str, dest_dir: str) -> str: ''' Channel will take care of moving the file from source to the destination diff --git a/parsl/channels/local/local.py b/parsl/channels/local/local.py index f93ca30e4b..e8e27afc41 100644 --- a/parsl/channels/local/local.py +++ b/parsl/channels/local/local.py @@ -128,13 +128,3 @@ def makedirs(self, path, mode=0o700, exist_ok=False): """ return os.makedirs(path, mode, exist_ok) - - @property - def script_dir(self): - return self._script_dir - - @script_dir.setter - def script_dir(self, value): - if value is not None: - value = os.path.abspath(value) - self._script_dir = value From 6323a00699a9ea5479807df13ea1ef514ff71b27 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 12:31:53 +0000 Subject: [PATCH 32/71] Remove channel hierarchy, leaving LocalChannel standalone --- parsl/channels/__init__.py | 4 - parsl/channels/base.py | 76 ------------------- parsl/channels/local/local.py | 3 +- parsl/dataflow/dflow.py | 4 +- parsl/providers/local/local.py | 2 +- .../test_providers/test_pbspro_template.py | 2 +- .../test_providers/test_slurm_template.py | 2 +- 7 files changed, 6 insertions(+), 87 deletions(-) delete mode 100644 parsl/channels/base.py diff --git a/parsl/channels/__init__.py b/parsl/channels/__init__.py index c81f6a8bf1..e69de29bb2 100644 --- a/parsl/channels/__init__.py +++ b/parsl/channels/__init__.py @@ -1,4 +0,0 @@ -from parsl.channels.base import Channel -from parsl.channels.local.local import LocalChannel - -__all__ = ['Channel', 'LocalChannel'] diff --git a/parsl/channels/base.py b/parsl/channels/base.py deleted file mode 100644 index 6316ad6287..0000000000 --- a/parsl/channels/base.py +++ /dev/null @@ -1,76 +0,0 @@ -from abc import ABCMeta, abstractmethod -from typing import Dict, Tuple - - -class Channel(metaclass=ABCMeta): - """This is a base class for a feature that has now been removed from - Parsl. This class should go away / be merged downwards into - LocalChannel. - """ - - # Script dir must be an absolute path - # This is initialized to None, which violates the type specification, - # and so code using script_dir must make sure it doesn't expect this - # to be not-None until after the DFK has initialized the channel - script_dir: str - - @abstractmethod - def execute_wait(self, cmd: str, walltime: int = 0, envs: Dict[str, str] = {}) -> 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) - ''' - pass - - @abstractmethod - def push_file(self, source: str, dest_dir: str) -> str: - ''' Channel will take care of moving the file from source to the destination - directory - - Args: - source (string) : Full filepath of the file to be moved - dest_dir (string) : Absolute path of the directory to move to - - Returns: - destination_path (string) - ''' - pass - - @abstractmethod - def pull_file(self, remote_source: str, local_dir: str) -> str: - ''' Transport file on the remote side to a local directory - - Args: - remote_source (string): remote_source - local_dir (string): Local directory to copy to - - - Returns: - destination_path (string) - ''' - pass - - @abstractmethod - def makedirs(self, path: str, mode: int = 0o511, exist_ok: bool = False) -> None: - """Create a directory. - - If intermediate directories do not exist, they will be created. - - Parameters - ---------- - path : str - Path of directory to create. - mode : int - Permissions (posix-style) for the newly-created directory. - exist_ok : bool - If False, raise an OSError if the target directory already exists. - """ - pass diff --git a/parsl/channels/local/local.py b/parsl/channels/local/local.py index e8e27afc41..d1e00570fc 100644 --- a/parsl/channels/local/local.py +++ b/parsl/channels/local/local.py @@ -4,14 +4,13 @@ import shutil import subprocess -from parsl.channels.base import Channel from parsl.channels.errors import FileCopyException from parsl.utils import RepresentationMixin logger = logging.getLogger(__name__) -class LocalChannel(Channel, RepresentationMixin): +class LocalChannel(RepresentationMixin): ''' This is not even really a channel, since opening a local shell is not heavy and done so infrequently that they do not need a persistent channel ''' diff --git a/parsl/dataflow/dflow.py b/parsl/dataflow/dflow.py index d377b32e9e..c89f8593a2 100644 --- a/parsl/dataflow/dflow.py +++ b/parsl/dataflow/dflow.py @@ -24,7 +24,7 @@ import parsl from parsl.app.errors import RemoteExceptionWrapper from parsl.app.futures import DataFuture -from parsl.channels import Channel +from parsl.channels.local.local import LocalChannel from parsl.config import Config from parsl.data_provider.data_manager import DataManager from parsl.data_provider.files import File @@ -1141,7 +1141,7 @@ def log_task_states(self) -> None: logger.info("End of summary") - def _create_script_dir(self, channel: Channel) -> None: + def _create_script_dir(self, channel: LocalChannel) -> None: """Create script directories across a channel Parameters diff --git a/parsl/providers/local/local.py b/parsl/providers/local/local.py index 221b97b532..adc7fb8fff 100644 --- a/parsl/providers/local/local.py +++ b/parsl/providers/local/local.py @@ -2,7 +2,7 @@ import os import time -from parsl.channels import LocalChannel +from parsl.channels.local.local import LocalChannel from parsl.jobs.states import JobState, JobStatus from parsl.launchers import SingleNodeLauncher from parsl.providers.base import ExecutionProvider diff --git a/parsl/tests/test_providers/test_pbspro_template.py b/parsl/tests/test_providers/test_pbspro_template.py index dec987ccbb..598d0c7928 100644 --- a/parsl/tests/test_providers/test_pbspro_template.py +++ b/parsl/tests/test_providers/test_pbspro_template.py @@ -3,7 +3,7 @@ import pytest -from parsl.channels import LocalChannel +from parsl.channels.local.local import LocalChannel from parsl.providers import PBSProProvider diff --git a/parsl/tests/test_providers/test_slurm_template.py b/parsl/tests/test_providers/test_slurm_template.py index 57fd8e4d0b..79f1113e40 100644 --- a/parsl/tests/test_providers/test_slurm_template.py +++ b/parsl/tests/test_providers/test_slurm_template.py @@ -4,7 +4,7 @@ import pytest -from parsl.channels import LocalChannel +from parsl.channels.local.local import LocalChannel from parsl.providers import SlurmProvider From b668a7e7ff22b6e3796e475061737fa05408ae5b Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 12:31:53 +0000 Subject: [PATCH 33/71] Remove channel hierarchy, leaving LocalChannel standalone --- parsl/channels/__init__.py | 4 - parsl/channels/base.py | 76 ------------------- parsl/channels/local/local.py | 3 +- parsl/dataflow/dflow.py | 8 +- parsl/providers/local/local.py | 2 +- .../test_providers/test_pbspro_template.py | 2 +- .../test_providers/test_slurm_template.py | 2 +- 7 files changed, 8 insertions(+), 89 deletions(-) delete mode 100644 parsl/channels/base.py diff --git a/parsl/channels/__init__.py b/parsl/channels/__init__.py index c81f6a8bf1..e69de29bb2 100644 --- a/parsl/channels/__init__.py +++ b/parsl/channels/__init__.py @@ -1,4 +0,0 @@ -from parsl.channels.base import Channel -from parsl.channels.local.local import LocalChannel - -__all__ = ['Channel', 'LocalChannel'] diff --git a/parsl/channels/base.py b/parsl/channels/base.py deleted file mode 100644 index 6316ad6287..0000000000 --- a/parsl/channels/base.py +++ /dev/null @@ -1,76 +0,0 @@ -from abc import ABCMeta, abstractmethod -from typing import Dict, Tuple - - -class Channel(metaclass=ABCMeta): - """This is a base class for a feature that has now been removed from - Parsl. This class should go away / be merged downwards into - LocalChannel. - """ - - # Script dir must be an absolute path - # This is initialized to None, which violates the type specification, - # and so code using script_dir must make sure it doesn't expect this - # to be not-None until after the DFK has initialized the channel - script_dir: str - - @abstractmethod - def execute_wait(self, cmd: str, walltime: int = 0, envs: Dict[str, str] = {}) -> 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) - ''' - pass - - @abstractmethod - def push_file(self, source: str, dest_dir: str) -> str: - ''' Channel will take care of moving the file from source to the destination - directory - - Args: - source (string) : Full filepath of the file to be moved - dest_dir (string) : Absolute path of the directory to move to - - Returns: - destination_path (string) - ''' - pass - - @abstractmethod - def pull_file(self, remote_source: str, local_dir: str) -> str: - ''' Transport file on the remote side to a local directory - - Args: - remote_source (string): remote_source - local_dir (string): Local directory to copy to - - - Returns: - destination_path (string) - ''' - pass - - @abstractmethod - def makedirs(self, path: str, mode: int = 0o511, exist_ok: bool = False) -> None: - """Create a directory. - - If intermediate directories do not exist, they will be created. - - Parameters - ---------- - path : str - Path of directory to create. - mode : int - Permissions (posix-style) for the newly-created directory. - exist_ok : bool - If False, raise an OSError if the target directory already exists. - """ - pass diff --git a/parsl/channels/local/local.py b/parsl/channels/local/local.py index e8e27afc41..d1e00570fc 100644 --- a/parsl/channels/local/local.py +++ b/parsl/channels/local/local.py @@ -4,14 +4,13 @@ import shutil import subprocess -from parsl.channels.base import Channel from parsl.channels.errors import FileCopyException from parsl.utils import RepresentationMixin logger = logging.getLogger(__name__) -class LocalChannel(Channel, RepresentationMixin): +class LocalChannel(RepresentationMixin): ''' This is not even really a channel, since opening a local shell is not heavy and done so infrequently that they do not need a persistent channel ''' diff --git a/parsl/dataflow/dflow.py b/parsl/dataflow/dflow.py index d377b32e9e..686eaf746b 100644 --- a/parsl/dataflow/dflow.py +++ b/parsl/dataflow/dflow.py @@ -24,7 +24,7 @@ import parsl from parsl.app.errors import RemoteExceptionWrapper from parsl.app.futures import DataFuture -from parsl.channels import Channel +from parsl.channels.local.local import LocalChannel from parsl.config import Config from parsl.data_provider.data_manager import DataManager from parsl.data_provider.files import File @@ -1141,13 +1141,13 @@ def log_task_states(self) -> None: logger.info("End of summary") - def _create_script_dir(self, channel: Channel) -> None: + def _create_script_dir(self, channel: LocalChannel) -> None: """Create script directories across a channel Parameters ---------- - channel: Channel obj - Channel over which the remote dirs are to be created + channel: LocalChannel + LocalChannel over which the script dir is to be created """ run_dir = self.run_dir diff --git a/parsl/providers/local/local.py b/parsl/providers/local/local.py index 221b97b532..adc7fb8fff 100644 --- a/parsl/providers/local/local.py +++ b/parsl/providers/local/local.py @@ -2,7 +2,7 @@ import os import time -from parsl.channels import LocalChannel +from parsl.channels.local.local import LocalChannel from parsl.jobs.states import JobState, JobStatus from parsl.launchers import SingleNodeLauncher from parsl.providers.base import ExecutionProvider diff --git a/parsl/tests/test_providers/test_pbspro_template.py b/parsl/tests/test_providers/test_pbspro_template.py index dec987ccbb..598d0c7928 100644 --- a/parsl/tests/test_providers/test_pbspro_template.py +++ b/parsl/tests/test_providers/test_pbspro_template.py @@ -3,7 +3,7 @@ import pytest -from parsl.channels import LocalChannel +from parsl.channels.local.local import LocalChannel from parsl.providers import PBSProProvider diff --git a/parsl/tests/test_providers/test_slurm_template.py b/parsl/tests/test_providers/test_slurm_template.py index 57fd8e4d0b..79f1113e40 100644 --- a/parsl/tests/test_providers/test_slurm_template.py +++ b/parsl/tests/test_providers/test_slurm_template.py @@ -4,7 +4,7 @@ import pytest -from parsl.channels import LocalChannel +from parsl.channels.local.local import LocalChannel from parsl.providers import SlurmProvider From dcff52325b56ab2eaa93be9e37f2eeab7e4a5df6 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 12:49:39 +0000 Subject: [PATCH 34/71] Tidyup and re-enable a LocalChannel test Previously this was in the /integration/ test directory, which is skipped when running automatic tests (due to a filter in conftest.py) --- .../test_channels/test_local_channel.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) rename parsl/tests/{integration => }/test_channels/test_local_channel.py (93%) diff --git a/parsl/tests/integration/test_channels/test_local_channel.py b/parsl/tests/test_channels/test_local_channel.py similarity index 93% rename from parsl/tests/integration/test_channels/test_local_channel.py rename to parsl/tests/test_channels/test_local_channel.py index cff403a611..e109cd841c 100644 --- a/parsl/tests/integration/test_channels/test_local_channel.py +++ b/parsl/tests/test_channels/test_local_channel.py @@ -1,6 +1,9 @@ +import pytest + from parsl.channels.local.local import LocalChannel +@pytest.mark.local def test_env(): ''' Regression testing for issue #27 ''' @@ -18,6 +21,7 @@ def test_env(): print("RC:{} \nSTDOUT:{} \nSTDERR:{}".format(rc, stdout, stderr)) +@pytest.mark.local def test_env_mod(): ''' Testing for env update at execute time. ''' @@ -34,9 +38,3 @@ def test_env_mod(): x = [s for s in stdout if s.startswith("TEST_ENV=fooo")] assert x, "User set env missing" - - -if __name__ == "__main__": - - test_env() - test_env_mod() From d13129c521ba849dab4ff63641ad42e9af8353b1 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 13:00:15 +0000 Subject: [PATCH 35/71] Remove broken local channel test The user channel cannot take None as its first parameter, userhome, so this test always fails. I suspect this test has been broken and not-run at least as far back as 2018, based on a few minutes look through history. But I'm not going to verify that. --- .../integration/test_channels/test_channels.py | 17 ----------------- 1 file changed, 17 deletions(-) delete mode 100644 parsl/tests/integration/test_channels/test_channels.py diff --git a/parsl/tests/integration/test_channels/test_channels.py b/parsl/tests/integration/test_channels/test_channels.py deleted file mode 100644 index f5943940eb..0000000000 --- a/parsl/tests/integration/test_channels/test_channels.py +++ /dev/null @@ -1,17 +0,0 @@ -from parsl.channels.local.local import LocalChannel - - -def test_local(): - - channel = LocalChannel(None, None) - - ec, out, err = channel.execute_wait('echo "pwd: $PWD"', 2) - - assert ec == 0, "Channel execute failed" - print("Stdout: ", out) - print("Stderr: ", err) - - -if __name__ == "__main__": - - test_local() From 26ef39a0f3cfd8321c797d83403a6a1569191a6e Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 13:11:53 +0000 Subject: [PATCH 36/71] Remove more unused integration/test_channels --- parsl/tests/integration/test_channels/__init__.py | 0 parsl/tests/integration/test_channels/remote_run.sh | 5 ----- 2 files changed, 5 deletions(-) delete mode 100644 parsl/tests/integration/test_channels/__init__.py delete mode 100644 parsl/tests/integration/test_channels/remote_run.sh diff --git a/parsl/tests/integration/test_channels/__init__.py b/parsl/tests/integration/test_channels/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/parsl/tests/integration/test_channels/remote_run.sh b/parsl/tests/integration/test_channels/remote_run.sh deleted file mode 100644 index aa4945d859..0000000000 --- a/parsl/tests/integration/test_channels/remote_run.sh +++ /dev/null @@ -1,5 +0,0 @@ -#!/bin/bash -echo "Hostname: $HOSTNAME" -echo "Cpu info -----" -cat /proc/cpuinfo -echo "Done----------" From 0b96f5f68bdb572bede6cecb8e6309dcb000f75e Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 13:16:21 +0000 Subject: [PATCH 37/71] fixup tests due to earlier commit, but they break in different ways now --- parsl/tests/test_providers/test_pbspro_template.py | 3 +-- parsl/tests/test_providers/test_slurm_template.py | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/parsl/tests/test_providers/test_pbspro_template.py b/parsl/tests/test_providers/test_pbspro_template.py index 598d0c7928..ed8ef8e4a2 100644 --- a/parsl/tests/test_providers/test_pbspro_template.py +++ b/parsl/tests/test_providers/test_pbspro_template.py @@ -3,7 +3,6 @@ import pytest -from parsl.channels.local.local import LocalChannel from parsl.providers import PBSProProvider @@ -12,7 +11,7 @@ def test_submit_script_basic(tmp_path): """Test slurm resources table""" provider = PBSProProvider( - queue="debug", channel=LocalChannel() + queue="debug" ) provider.script_dir = tmp_path provider.channel.script_dir = tmp_path diff --git a/parsl/tests/test_providers/test_slurm_template.py b/parsl/tests/test_providers/test_slurm_template.py index 79f1113e40..5c36026bfb 100644 --- a/parsl/tests/test_providers/test_slurm_template.py +++ b/parsl/tests/test_providers/test_slurm_template.py @@ -4,7 +4,6 @@ import pytest -from parsl.channels.local.local import LocalChannel from parsl.providers import SlurmProvider @@ -13,7 +12,7 @@ def test_submit_script_basic(tmp_path): """Test slurm resources table""" provider = SlurmProvider( - partition="debug", channel=LocalChannel() + partition="debug" ) provider.script_dir = tmp_path provider.channel.script_dir = tmp_path From 8ceaa10585fffefb9191a3e4d9bb4c6851e59da1 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 13:17:57 +0000 Subject: [PATCH 38/71] fix tests broken in parent commit, removing channel configurability --- parsl/tests/test_providers/test_pbspro_template.py | 2 +- parsl/tests/test_providers/test_slurm_template.py | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/parsl/tests/test_providers/test_pbspro_template.py b/parsl/tests/test_providers/test_pbspro_template.py index dec987ccbb..9d0e01b944 100644 --- a/parsl/tests/test_providers/test_pbspro_template.py +++ b/parsl/tests/test_providers/test_pbspro_template.py @@ -12,7 +12,7 @@ def test_submit_script_basic(tmp_path): """Test slurm resources table""" provider = PBSProProvider( - queue="debug", channel=LocalChannel() + queue="debug" ) provider.script_dir = tmp_path provider.channel.script_dir = tmp_path diff --git a/parsl/tests/test_providers/test_slurm_template.py b/parsl/tests/test_providers/test_slurm_template.py index 57fd8e4d0b..5c36026bfb 100644 --- a/parsl/tests/test_providers/test_slurm_template.py +++ b/parsl/tests/test_providers/test_slurm_template.py @@ -4,7 +4,6 @@ import pytest -from parsl.channels import LocalChannel from parsl.providers import SlurmProvider @@ -13,7 +12,7 @@ def test_submit_script_basic(tmp_path): """Test slurm resources table""" provider = SlurmProvider( - partition="debug", channel=LocalChannel() + partition="debug" ) provider.script_dir = tmp_path provider.channel.script_dir = tmp_path From 278660dd3a9de61075e70b2dd8cb29c7f4b26086 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 13:23:53 +0000 Subject: [PATCH 39/71] Add a debug line to understand an if statement that behaves differently in an upcoming commit --- parsl/channels/local/local.py | 1 + 1 file changed, 1 insertion(+) diff --git a/parsl/channels/local/local.py b/parsl/channels/local/local.py index f93ca30e4b..7fd3d9927b 100644 --- a/parsl/channels/local/local.py +++ b/parsl/channels/local/local.py @@ -96,6 +96,7 @@ def push_file(self, source, dest_dir): local_dest = os.path.join(dest_dir, os.path.basename(source)) # Only attempt to copy if the target dir and source dir are different + logger.debug(f"push file comparing directories: {os.path.dirname(source)} vs {dest_dir}") if os.path.dirname(source) != dest_dir: try: shutil.copyfile(source, local_dest) From 94675c67ee440f45930f6047c0dfa12161a500af Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 13:32:49 +0000 Subject: [PATCH 40/71] bugfix incorrect type that was causing equal paths to be unequal (because one was a str, one a posixfile) in localchannel push/pull code --- parsl/channels/local/local.py | 1 + parsl/dataflow/dflow.py | 4 +++- parsl/tests/test_providers/test_pbspro_template.py | 2 +- parsl/tests/test_providers/test_slurm_template.py | 2 +- 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/parsl/channels/local/local.py b/parsl/channels/local/local.py index 2c2072d75e..ff52c4d880 100644 --- a/parsl/channels/local/local.py +++ b/parsl/channels/local/local.py @@ -97,6 +97,7 @@ def push_file(self, source, dest_dir): # Only attempt to copy if the target dir and source dir are different logger.debug(f"push file comparing directories: {os.path.dirname(source)} vs {dest_dir}") + logger.debug(f"push file comparing directories: {os.path.dirname(source)!r} vs {dest_dir!r}") if os.path.dirname(source) != dest_dir: try: shutil.copyfile(source, local_dest) diff --git a/parsl/dataflow/dflow.py b/parsl/dataflow/dflow.py index d377b32e9e..2c2c10c2c9 100644 --- a/parsl/dataflow/dflow.py +++ b/parsl/dataflow/dflow.py @@ -1151,7 +1151,9 @@ def _create_script_dir(self, channel: Channel) -> None: """ run_dir = self.run_dir - channel.script_dir = os.path.join(run_dir, 'submit_scripts') + sd = os.path.join(run_dir, 'submit_scripts') + assert isinstance(sd, str) + channel.script_dir = sd channel.makedirs(channel.script_dir, exist_ok=True) diff --git a/parsl/tests/test_providers/test_pbspro_template.py b/parsl/tests/test_providers/test_pbspro_template.py index 9d0e01b944..b86ea44321 100644 --- a/parsl/tests/test_providers/test_pbspro_template.py +++ b/parsl/tests/test_providers/test_pbspro_template.py @@ -15,7 +15,7 @@ def test_submit_script_basic(tmp_path): queue="debug" ) provider.script_dir = tmp_path - provider.channel.script_dir = tmp_path + provider.channel.script_dir = str(tmp_path) job_id = str(random.randint(55000, 59000)) provider.execute_wait = mock.Mock(spec=PBSProProvider.execute_wait) provider.execute_wait.return_value = (0, job_id, "") diff --git a/parsl/tests/test_providers/test_slurm_template.py b/parsl/tests/test_providers/test_slurm_template.py index 5c36026bfb..765270913b 100644 --- a/parsl/tests/test_providers/test_slurm_template.py +++ b/parsl/tests/test_providers/test_slurm_template.py @@ -15,7 +15,7 @@ def test_submit_script_basic(tmp_path): partition="debug" ) provider.script_dir = tmp_path - provider.channel.script_dir = tmp_path + provider.channel.script_dir = str(tmp_path) job_id = str(random.randint(55000, 59000)) provider.execute_wait = mock.MagicMock(spec=SlurmProvider.execute_wait) provider.execute_wait.return_value = (0, f"Submitted batch job {job_id}", "") From ee5b9bd631317bcb7f8e6af5e9770597b86121d0 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 17:01:27 +0000 Subject: [PATCH 41/71] Move plugin doc for channel to a removed-abstractions section --- docs/userguide/plugins.rst | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/userguide/plugins.rst b/docs/userguide/plugins.rst index c3c38dea63..7d1eb81685 100644 --- a/docs/userguide/plugins.rst +++ b/docs/userguide/plugins.rst @@ -34,10 +34,6 @@ add on any wrappers that are needed to launch the command (eg srun inside slurm). Providers and launchers are usually paired together for a particular system type. -Parsl also has a deprecated ``Channel`` abstraction. See -`issue 3515 `_ -for further discussion. - File staging ------------ Parsl can copy input files from an arbitrary URL into a task's working @@ -101,3 +97,9 @@ such as tuples, lists, sets and dicts. This plugin interface might be used to interface other task-like or future-like objects to the Parsl dependency mechanism, by describing how they can be interpreted as a Future. + +Removed interfaces +------------------ + +Parsl had a ``Channel`` interface which was removed in 2024. See +`issue 3515 `_ for further discussion. From e79427c383706317805e2bfbd2dc557050dce208 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 17:10:01 +0000 Subject: [PATCH 42/71] Combine provider and channel script dir initialization and creation This is on the way to removing channel script dir entirely The channel script dir will not differ from the provider script dir any more - they were different because one was on the remote file system and one was on the local filesystem. Future code modifications can be clearer that these two are the same. --- parsl/dataflow/dflow.py | 25 ++++--------------------- 1 file changed, 4 insertions(+), 21 deletions(-) diff --git a/parsl/dataflow/dflow.py b/parsl/dataflow/dflow.py index c6e20eb947..e8bf107a78 100644 --- a/parsl/dataflow/dflow.py +++ b/parsl/dataflow/dflow.py @@ -24,7 +24,6 @@ import parsl from parsl.app.errors import RemoteExceptionWrapper from parsl.app.futures import DataFuture -from parsl.channels.local.local import LocalChannel from parsl.config import Config from parsl.data_provider.data_manager import DataManager from parsl.data_provider.files import File @@ -1141,22 +1140,6 @@ def log_task_states(self) -> None: logger.info("End of summary") - def _create_script_dir(self, channel: LocalChannel) -> None: - """Create script directories across a channel - - Parameters - ---------- - channel: LocalChannel - LocalChannel over which the script dir is to be created - """ - run_dir = self.run_dir - - sd = os.path.join(run_dir, 'submit_scripts') - assert isinstance(sd, str) - channel.script_dir = sd - - channel.makedirs(channel.script_dir, exist_ok=True) - def add_executors(self, executors: Sequence[ParslExecutor]) -> None: for executor in executors: executor.run_id = self.run_id @@ -1167,10 +1150,10 @@ def add_executors(self, executors: Sequence[ParslExecutor]) -> None: executor.submit_monitoring_radio = self.monitoring.radio if hasattr(executor, 'provider'): if hasattr(executor.provider, 'script_dir'): - executor.provider.script_dir = os.path.join(self.run_dir, 'submit_scripts') - os.makedirs(executor.provider.script_dir, exist_ok=True) - - self._create_script_dir(executor.provider.channel) + script_dir = os.path.join(self.run_dir, 'submit_scripts') + executor.provider.script_dir = script_dir + executor.provider.channel.script_dir = script_dir + os.makedirs(script_dir, exist_ok=True) self.executors[executor.label] = executor executor.start() From 054668fbfb1788f1c91664ab2bf523c20281451a Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 17:22:05 +0000 Subject: [PATCH 43/71] Remove file moving from LocalProvider --- parsl/providers/local/local.py | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/parsl/providers/local/local.py b/parsl/providers/local/local.py index adc7fb8fff..13b4899e53 100644 --- a/parsl/providers/local/local.py +++ b/parsl/providers/local/local.py @@ -32,9 +32,6 @@ class LocalProvider(ExecutionProvider, RepresentationMixin): Ratio of provisioned task slots to active tasks. A parallelism value of 1 represents aggressive scaling where as many resources as possible are used; parallelism close to 0 represents the opposite situation in which as few resources as possible (i.e., min_blocks) are used. - move_files : Optional[Bool] - Should files be moved? By default, Parsl will try to figure this out itself (= None). - If True, then will always move. If False, will never move. worker_init : str Command to be run before starting a worker, such as 'module load Anaconda; source activate env'. """ @@ -47,8 +44,7 @@ def __init__(self, max_blocks=1, worker_init='', cmd_timeout=30, - parallelism=1, - move_files=None): + parallelism=1): self.channel = LocalChannel() self._label = 'local' self.nodes_per_block = nodes_per_block @@ -60,7 +56,6 @@ def __init__(self, self.parallelism = parallelism self.script_dir = None self.cmd_timeout = cmd_timeout - self.move_files = move_files # Dictionary that keeps track of jobs, keyed on job_id self.resources = {} @@ -82,7 +77,6 @@ def status(self, job_ids): if job_dict['status'] and job_dict['status'].terminal: # We already checked this and it can't change after that continue - # Script path should point to remote path if _should_move_files() is True script_path = job_dict['script_path'] alive = self._is_alive(job_dict) @@ -136,8 +130,6 @@ def _is_alive(self, job_dict): def _job_file_path(self, script_path: str, suffix: str) -> str: path = '{0}{1}'.format(script_path, suffix) - if self._should_move_files(): - path = self.channel.pull_file(path, self.script_dir) return path def _read_job_file(self, script_path: str, suffix: str) -> str: @@ -215,9 +207,6 @@ def submit(self, command, tasks_per_node, job_name="parsl.localprovider"): job_id = None remote_pid = None - if self._should_move_files(): - logger.debug("Pushing start script") - script_path = self.channel.push_file(script_path, self.channel.script_dir) logger.debug("Launching") # We need to capture the exit code and the streams, so we put them in files. We also write @@ -253,9 +242,6 @@ def submit(self, command, tasks_per_node, job_name="parsl.localprovider"): return job_id - def _should_move_files(self): - return (self.move_files is None and not isinstance(self.channel, LocalChannel)) or (self.move_files) - def cancel(self, job_ids): ''' Cancels the jobs specified by a list of job ids From d8487ffed60694be76bf2869b8f62ae3f5004507 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 17:26:27 +0000 Subject: [PATCH 44/71] Remove file moving from SlurmProvider --- parsl/providers/slurm/slurm.py | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/parsl/providers/slurm/slurm.py b/parsl/providers/slurm/slurm.py index ad87260cdc..be408a66fd 100644 --- a/parsl/providers/slurm/slurm.py +++ b/parsl/providers/slurm/slurm.py @@ -103,7 +103,6 @@ class SlurmProvider(ClusterProvider, RepresentationMixin): :class:`~parsl.launchers.SingleNodeLauncher` (the default), :class:`~parsl.launchers.SrunLauncher`, or :class:`~parsl.launchers.AprunLauncher` - move_files : Optional[Bool]: should files be moved? by default, Parsl will try to move files. """ @typeguard.typechecked @@ -125,7 +124,6 @@ def __init__(self, worker_init: str = '', cmd_timeout: int = 10, exclusive: bool = True, - move_files: bool = True, launcher: Launcher = SingleNodeLauncher()): label = 'slurm' super().__init__(label, @@ -142,7 +140,6 @@ def __init__(self, self.cores_per_node = cores_per_node self.mem_per_node = mem_per_node self.exclusive = exclusive - self.move_files = move_files self.account = account self.qos = qos self.constraint = constraint @@ -287,14 +284,7 @@ def submit(self, command: str, tasks_per_node: int, job_name="parsl.slurm") -> s logger.debug("Writing submit script") self._write_submit_script(template_string, script_path, job_name, job_config) - if self.move_files: - logger.debug("moving files") - channel_script_path = self.channel.push_file(script_path, self.channel.script_dir) - else: - logger.debug("not moving files") - channel_script_path = script_path - - retcode, stdout, stderr = self.execute_wait("sbatch {0}".format(channel_script_path)) + retcode, stdout, stderr = self.execute_wait("sbatch {0}".format(script_path)) if retcode == 0: for line in stdout.split('\n'): From e95036d459b6bd8b2a9fe1eca72aa5156e34499e Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 17:27:55 +0000 Subject: [PATCH 45/71] Use slurm provider script dir instead of channel script dir -- these should be the same --- parsl/providers/slurm/slurm.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/parsl/providers/slurm/slurm.py b/parsl/providers/slurm/slurm.py index be408a66fd..3538fa6b95 100644 --- a/parsl/providers/slurm/slurm.py +++ b/parsl/providers/slurm/slurm.py @@ -3,7 +3,7 @@ import os import re import time -from typing import Optional +from typing import Any, Dict, Optional import typeguard @@ -265,8 +265,8 @@ def submit(self, command: str, tasks_per_node: int, job_name="parsl.slurm") -> s logger.debug("Requesting one block with {} nodes".format(self.nodes_per_block)) - job_config = {} - job_config["submit_script_dir"] = self.channel.script_dir + job_config: Dict[str, Any] = {} + job_config["submit_script_dir"] = self.script_dir job_config["nodes"] = self.nodes_per_block job_config["tasks_per_node"] = tasks_per_node job_config["walltime"] = wtime_to_minutes(self.walltime) From 0528dbedf512bad50c48b16720c5c0cb8299462e Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 17:32:47 +0000 Subject: [PATCH 46/71] Untested, remove file movement from PBS providers x 2 --- parsl/providers/pbspro/pbspro.py | 4 +--- parsl/providers/torque/torque.py | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/parsl/providers/pbspro/pbspro.py b/parsl/providers/pbspro/pbspro.py index efaaf7ec01..0822a27442 100644 --- a/parsl/providers/pbspro/pbspro.py +++ b/parsl/providers/pbspro/pbspro.py @@ -178,15 +178,13 @@ def submit(self, command, tasks_per_node, job_name="parsl"): logger.debug("Writing submit script") self._write_submit_script(self.template_string, script_path, job_name, job_config) - channel_script_path = self.channel.push_file(script_path, self.channel.script_dir) - submit_options = '' if self.queue is not None: submit_options = '{0} -q {1}'.format(submit_options, self.queue) if self.account is not None: submit_options = '{0} -A {1}'.format(submit_options, self.account) - launch_cmd = "qsub {0} {1}".format(submit_options, channel_script_path) + launch_cmd = "qsub {0} {1}".format(submit_options, script_path) retcode, stdout, stderr = self.execute_wait(launch_cmd) job_id = None diff --git a/parsl/providers/torque/torque.py b/parsl/providers/torque/torque.py index 44c64e1cc1..8bdf0ec893 100644 --- a/parsl/providers/torque/torque.py +++ b/parsl/providers/torque/torque.py @@ -184,15 +184,13 @@ def submit(self, command, tasks_per_node, job_name="parsl.torque"): logger.debug("Writing submit script") self._write_submit_script(self.template_string, script_path, job_name, job_config) - channel_script_path = self.channel.push_file(script_path, self.channel.script_dir) - submit_options = '' if self.queue is not None: submit_options = '{0} -q {1}'.format(submit_options, self.queue) if self.account is not None: submit_options = '{0} -A {1}'.format(submit_options, self.account) - launch_cmd = "qsub {0} {1}".format(submit_options, channel_script_path) + launch_cmd = "qsub {0} {1}".format(submit_options, script_path) retcode, stdout, stderr = self.execute_wait(launch_cmd) job_id = None From b6b8e397fa84f8e3ab114e881852583aa60c9e9d Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 17:33:42 +0000 Subject: [PATCH 47/71] Remove push_file from lsf provider --- parsl/providers/lsf/lsf.py | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/parsl/providers/lsf/lsf.py b/parsl/providers/lsf/lsf.py index 972fbd49b7..152b71db74 100644 --- a/parsl/providers/lsf/lsf.py +++ b/parsl/providers/lsf/lsf.py @@ -65,7 +65,6 @@ class LSFProvider(ClusterProvider, RepresentationMixin): :class:`~parsl.launchers.SingleNodeLauncher` (the default), :class:`~parsl.launchers.SrunLauncher`, or :class:`~parsl.launchers.AprunLauncher` - move_files : Optional[Bool]: should files be moved? by default, Parsl will try to move files. bsub_redirection: Bool Should a redirection symbol "<" be included when submitting jobs, i.e., Bsub < job_script. request_by_nodes: Bool @@ -88,7 +87,6 @@ def __init__(self, project=None, queue=None, cmd_timeout=120, - move_files=True, bsub_redirection=False, request_by_nodes=True, launcher=SingleNodeLauncher()): @@ -107,7 +105,6 @@ def __init__(self, self.queue = queue self.cores_per_block = cores_per_block self.cores_per_node = cores_per_node - self.move_files = move_files self.bsub_redirection = bsub_redirection self.request_by_nodes = request_by_nodes @@ -225,17 +222,10 @@ def submit(self, command, tasks_per_node, job_name="parsl.lsf"): logger.debug("Writing submit script") self._write_submit_script(template_string, script_path, job_name, job_config) - if self.move_files: - logger.debug("moving files") - channel_script_path = self.channel.push_file(script_path, self.channel.script_dir) - else: - logger.debug("not moving files") - channel_script_path = script_path - if self.bsub_redirection: - cmd = "bsub < {0}".format(channel_script_path) + cmd = "bsub < {0}".format(script_path) else: - cmd = "bsub {0}".format(channel_script_path) + cmd = "bsub {0}".format(script_path) retcode, stdout, stderr = super().execute_wait(cmd) job_id = None From fd2d70ca4bb81791366a98d647d1053aee3422b1 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 17:34:20 +0000 Subject: [PATCH 48/71] Remove push_file from grid engine provider --- parsl/providers/grid_engine/grid_engine.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/parsl/providers/grid_engine/grid_engine.py b/parsl/providers/grid_engine/grid_engine.py index 96bf5ee375..fd6ab4ab95 100644 --- a/parsl/providers/grid_engine/grid_engine.py +++ b/parsl/providers/grid_engine/grid_engine.py @@ -137,11 +137,10 @@ def submit(self, command, tasks_per_node, job_name="parsl.sge"): logger.debug("Writing submit script") self._write_submit_script(template_string, script_path, job_name, job_config) - channel_script_path = self.channel.push_file(script_path, self.channel.script_dir) if self.queue is not None: - cmd = "qsub -q {0} -terse {1}".format(self.queue, channel_script_path) + cmd = "qsub -q {0} -terse {1}".format(self.queue, script_path) else: - cmd = "qsub -terse {0}".format(channel_script_path) + cmd = "qsub -terse {0}".format(script_path) retcode, stdout, stderr = self.execute_wait(cmd) if retcode == 0: From 83c1d2f583b3ae5e7b58b0c7397a552aade9e5e8 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 17:35:15 +0000 Subject: [PATCH 49/71] Remove push_file from condor provider --- parsl/providers/condor/condor.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/parsl/providers/condor/condor.py b/parsl/providers/condor/condor.py index 063233f0ae..1d0b1cba23 100644 --- a/parsl/providers/condor/condor.py +++ b/parsl/providers/condor/condor.py @@ -238,10 +238,9 @@ def submit(self, command, tasks_per_node, job_name="parsl.condor"): with open(userscript_path, 'w') as f: f.write(job_config["worker_init"] + '\n' + wrapped_command) - user_script_path = self.channel.push_file(userscript_path, self.channel.script_dir) - the_input_files = [user_script_path] + self.transfer_input_files + the_input_files = [userscript_path] + self.transfer_input_files job_config["input_files"] = ','.join(the_input_files) - job_config["job_script"] = os.path.basename(user_script_path) + job_config["job_script"] = os.path.basename(userscript_path) # Construct and move the submit script self._write_submit_script(template_string, script_path, job_name, job_config) From 631e0d83c3fa945f92d21d84608dd4d88f7e7b96 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 17:35:50 +0000 Subject: [PATCH 50/71] Remove push_file from cobalt provider --- parsl/providers/cobalt/cobalt.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/parsl/providers/cobalt/cobalt.py b/parsl/providers/cobalt/cobalt.py index 6a6c446da9..2ad6938696 100644 --- a/parsl/providers/cobalt/cobalt.py +++ b/parsl/providers/cobalt/cobalt.py @@ -171,10 +171,8 @@ def submit(self, command, tasks_per_node, job_name="parsl.cobalt"): logger.debug("Writing submit script") self._write_submit_script(template_string, script_path, job_name, job_config) - channel_script_path = self.channel.push_file(script_path, self.channel.script_dir) - command = 'qsub -n {0} {1} -t {2} {3} {4}'.format( - self.nodes_per_block, queue_opt, wtime_to_minutes(self.walltime), account_opt, channel_script_path) + self.nodes_per_block, queue_opt, wtime_to_minutes(self.walltime), account_opt, script_path) logger.debug("Executing {}".format(command)) retcode, stdout, stderr = self.execute_wait(command) From c2f2d0678d22f4036e721834bad2926a5f076102 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 17:37:14 +0000 Subject: [PATCH 51/71] fix condor file movement commit 2 commits back --- parsl/providers/condor/condor.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/parsl/providers/condor/condor.py b/parsl/providers/condor/condor.py index 1d0b1cba23..adbeaadb8e 100644 --- a/parsl/providers/condor/condor.py +++ b/parsl/providers/condor/condor.py @@ -244,9 +244,8 @@ def submit(self, command, tasks_per_node, job_name="parsl.condor"): # Construct and move the submit script self._write_submit_script(template_string, script_path, job_name, job_config) - channel_script_path = self.channel.push_file(script_path, self.channel.script_dir) - cmd = "condor_submit {0}".format(channel_script_path) + cmd = "condor_submit {0}".format(script_path) try: retcode, stdout, stderr = self.execute_wait(cmd) except Exception as e: From 981046ed5d404505be8479b0d71c124ddb12af5f Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 17:38:20 +0000 Subject: [PATCH 52/71] Remove pull/push files methods --- docs/reference.rst | 1 - parsl/channels/errors.py | 14 ------------- parsl/channels/local/local.py | 38 ----------------------------------- 3 files changed, 53 deletions(-) delete mode 100644 parsl/channels/errors.py diff --git a/docs/reference.rst b/docs/reference.rst index fa3c8423de..eec543bd5c 100644 --- a/docs/reference.rst +++ b/docs/reference.rst @@ -170,7 +170,6 @@ Exceptions parsl.providers.errors.ScaleOutFailed parsl.providers.errors.SchedulerMissingArgs parsl.providers.errors.ScriptPathError - parsl.channels.errors.FileCopyException parsl.executors.high_throughput.errors.WorkerLost parsl.executors.high_throughput.interchange.ManagerLost parsl.serialize.errors.DeserializationError diff --git a/parsl/channels/errors.py b/parsl/channels/errors.py deleted file mode 100644 index 82107b387e..0000000000 --- a/parsl/channels/errors.py +++ /dev/null @@ -1,14 +0,0 @@ -''' Exceptions raise by Apps. -''' -from parsl.errors import ParslError - - -class FileCopyException(ParslError): - ''' File copy operation failed - - Contains: - e (parent exception object) - ''' - - def __init__(self, e: Exception) -> None: - super().__init__("File copy failed due to {0}".format(e)) diff --git a/parsl/channels/local/local.py b/parsl/channels/local/local.py index 468c5233f6..5e24289e91 100644 --- a/parsl/channels/local/local.py +++ b/parsl/channels/local/local.py @@ -1,10 +1,8 @@ import copy import logging import os -import shutil import subprocess -from parsl.channels.errors import FileCopyException from parsl.utils import RepresentationMixin logger = logging.getLogger(__name__) @@ -74,42 +72,6 @@ def execute_wait(self, cmd, walltime=None, envs={}): return (retcode, stdout.decode("utf-8"), stderr.decode("utf-8")) - def push_file(self, source, dest_dir): - ''' If the source files dirpath is the same as dest_dir, a copy - is not necessary, and nothing is done. Else a copy is made. - - Args: - - source (string) : Path to the source file - - dest_dir (string) : Path to the directory to which the files is to be copied - - Returns: - - destination_path (String) : Absolute path of the destination file - - Raises: - - FileCopyException : If file copy failed. - ''' - - local_dest = os.path.join(dest_dir, os.path.basename(source)) - - # Only attempt to copy if the target dir and source dir are different - logger.debug(f"push file comparing directories: {os.path.dirname(source)} vs {dest_dir}") - logger.debug(f"push file comparing directories: {os.path.dirname(source)!r} vs {dest_dir!r}") - if os.path.dirname(source) != dest_dir: - try: - shutil.copyfile(source, local_dest) - os.chmod(local_dest, 0o700) - - except OSError as e: - raise FileCopyException(e) - - else: - os.chmod(local_dest, 0o700) - - return local_dest - - def pull_file(self, remote_source, local_dir): - return self.push_file(remote_source, local_dir) - def makedirs(self, path, mode=0o700, exist_ok=False): """Create a directory. From 2a0c8ffdfb428f60fc0f8e7ec600b03f6d8d196a Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 17:44:03 +0000 Subject: [PATCH 53/71] remove channel.script_dir attribute entirely, replacing the remaining uses with provider.script_dir which should be the same --- parsl/channels/local/local.py | 4 +--- parsl/dataflow/dflow.py | 1 - parsl/providers/condor/condor.py | 2 +- parsl/providers/grid_engine/grid_engine.py | 2 +- parsl/providers/lsf/lsf.py | 2 +- parsl/providers/pbspro/pbspro.py | 2 +- parsl/providers/torque/torque.py | 2 +- parsl/tests/test_providers/test_pbspro_template.py | 1 - parsl/tests/test_providers/test_slurm_template.py | 1 - 9 files changed, 6 insertions(+), 11 deletions(-) diff --git a/parsl/channels/local/local.py b/parsl/channels/local/local.py index 5e24289e91..ba4f89e4b1 100644 --- a/parsl/channels/local/local.py +++ b/parsl/channels/local/local.py @@ -14,12 +14,11 @@ class LocalChannel(RepresentationMixin): ''' def __init__(self): - ''' Initialize the local channel. script_dir is required by set to a default. + ''' Initialize the local channel. KwArgs: - userhome (string): (default='.') This is provided as a way to override and set a specific userhome - envs (dict) : A dictionary of env variables to be set when launching the shell - - script_dir (string): Directory to place scripts ''' self.userhome = os.path.abspath(".") envs = {} @@ -27,7 +26,6 @@ def __init__(self): local_env = os.environ.copy() self._envs = copy.deepcopy(local_env) self._envs.update(envs) - self.script_dir = None def execute_wait(self, cmd, walltime=None, envs={}): ''' Synchronously execute a commandline string on the shell. diff --git a/parsl/dataflow/dflow.py b/parsl/dataflow/dflow.py index e8bf107a78..fe6c90099c 100644 --- a/parsl/dataflow/dflow.py +++ b/parsl/dataflow/dflow.py @@ -1152,7 +1152,6 @@ def add_executors(self, executors: Sequence[ParslExecutor]) -> None: if hasattr(executor.provider, 'script_dir'): script_dir = os.path.join(self.run_dir, 'submit_scripts') executor.provider.script_dir = script_dir - executor.provider.channel.script_dir = script_dir os.makedirs(script_dir, exist_ok=True) self.executors[executor.label] = executor diff --git a/parsl/providers/condor/condor.py b/parsl/providers/condor/condor.py index adbeaadb8e..150c72dfe7 100644 --- a/parsl/providers/condor/condor.py +++ b/parsl/providers/condor/condor.py @@ -219,7 +219,7 @@ def submit(self, command, tasks_per_node, job_name="parsl.condor"): job_config = {} job_config["job_name"] = job_name - job_config["submit_script_dir"] = self.channel.script_dir + job_config["submit_script_dir"] = self.script_dir job_config["project"] = self.project job_config["nodes"] = self.nodes_per_block job_config["scheduler_options"] = scheduler_options diff --git a/parsl/providers/grid_engine/grid_engine.py b/parsl/providers/grid_engine/grid_engine.py index fd6ab4ab95..b01e86cd5d 100644 --- a/parsl/providers/grid_engine/grid_engine.py +++ b/parsl/providers/grid_engine/grid_engine.py @@ -95,7 +95,7 @@ def get_configs(self, command, tasks_per_node): self.nodes_per_block, tasks_per_node)) job_config = {} - job_config["submit_script_dir"] = self.channel.script_dir + job_config["submit_script_dir"] = self.script_dir job_config["nodes"] = self.nodes_per_block job_config["walltime"] = self.walltime job_config["scheduler_options"] = self.scheduler_options diff --git a/parsl/providers/lsf/lsf.py b/parsl/providers/lsf/lsf.py index 152b71db74..f2d3f88652 100644 --- a/parsl/providers/lsf/lsf.py +++ b/parsl/providers/lsf/lsf.py @@ -206,7 +206,7 @@ def submit(self, command, tasks_per_node, job_name="parsl.lsf"): logger.debug("Requesting one block with {} nodes".format(self.nodes_per_block)) job_config = {} - job_config["submit_script_dir"] = self.channel.script_dir + job_config["submit_script_dir"] = self.script_dir job_config["nodes"] = self.nodes_per_block job_config["tasks_per_node"] = tasks_per_node job_config["walltime"] = wtime_to_minutes(self.walltime) diff --git a/parsl/providers/pbspro/pbspro.py b/parsl/providers/pbspro/pbspro.py index 0822a27442..aa2e5e2f54 100644 --- a/parsl/providers/pbspro/pbspro.py +++ b/parsl/providers/pbspro/pbspro.py @@ -154,7 +154,7 @@ def submit(self, command, tasks_per_node, job_name="parsl"): ) job_config = {} - job_config["submit_script_dir"] = self.channel.script_dir + job_config["submit_script_dir"] = self.script_dir job_config["nodes_per_block"] = self.nodes_per_block job_config["ncpus"] = self.cpus_per_node job_config["walltime"] = self.walltime diff --git a/parsl/providers/torque/torque.py b/parsl/providers/torque/torque.py index 8bdf0ec893..48a6f4d0d4 100644 --- a/parsl/providers/torque/torque.py +++ b/parsl/providers/torque/torque.py @@ -166,7 +166,7 @@ def submit(self, command, tasks_per_node, job_name="parsl.torque"): job_config = {} # TODO : script_path might need to change to accommodate script dir set via channels - job_config["submit_script_dir"] = self.channel.script_dir + job_config["submit_script_dir"] = self.script_dir job_config["nodes"] = self.nodes_per_block job_config["task_blocks"] = self.nodes_per_block * tasks_per_node job_config["nodes_per_block"] = self.nodes_per_block diff --git a/parsl/tests/test_providers/test_pbspro_template.py b/parsl/tests/test_providers/test_pbspro_template.py index 179ebbd690..1264731120 100644 --- a/parsl/tests/test_providers/test_pbspro_template.py +++ b/parsl/tests/test_providers/test_pbspro_template.py @@ -14,7 +14,6 @@ def test_submit_script_basic(tmp_path): queue="debug" ) provider.script_dir = tmp_path - provider.channel.script_dir = str(tmp_path) job_id = str(random.randint(55000, 59000)) provider.execute_wait = mock.Mock(spec=PBSProProvider.execute_wait) provider.execute_wait.return_value = (0, job_id, "") diff --git a/parsl/tests/test_providers/test_slurm_template.py b/parsl/tests/test_providers/test_slurm_template.py index 765270913b..55074fefe3 100644 --- a/parsl/tests/test_providers/test_slurm_template.py +++ b/parsl/tests/test_providers/test_slurm_template.py @@ -15,7 +15,6 @@ def test_submit_script_basic(tmp_path): partition="debug" ) provider.script_dir = tmp_path - provider.channel.script_dir = str(tmp_path) job_id = str(random.randint(55000, 59000)) provider.execute_wait = mock.MagicMock(spec=SlurmProvider.execute_wait) provider.execute_wait.return_value = (0, f"Submitted batch job {job_id}", "") From a9ecec6a7f17c930f247d96545f7d9ef3c57db29 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 17:46:44 +0000 Subject: [PATCH 54/71] Remove now-unused channel makedirs --- parsl/channels/local/local.py | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/parsl/channels/local/local.py b/parsl/channels/local/local.py index ba4f89e4b1..2b18b37b00 100644 --- a/parsl/channels/local/local.py +++ b/parsl/channels/local/local.py @@ -69,20 +69,3 @@ def execute_wait(self, cmd, walltime=None, envs={}): logger.debug("Execution of command in process %s completed normally", proc.pid) return (retcode, stdout.decode("utf-8"), stderr.decode("utf-8")) - - def makedirs(self, path, mode=0o700, exist_ok=False): - """Create a directory. - - If intermediate directories do not exist, they will be created. - - Parameters - ---------- - path : str - Path of directory to create. - mode : int - Permissions (posix-style) for the newly-created directory. - exist_ok : bool - If False, raise an OSError if the target directory already exists. - """ - - return os.makedirs(path, mode, exist_ok) From 994ff31d1158b160fda8dac4f99438a630d9ac0c Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 17:50:31 +0000 Subject: [PATCH 55/71] Remove unused userhome attribute of local channel --- parsl/channels/local/local.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/parsl/channels/local/local.py b/parsl/channels/local/local.py index 2b18b37b00..adb469e721 100644 --- a/parsl/channels/local/local.py +++ b/parsl/channels/local/local.py @@ -17,10 +17,8 @@ def __init__(self): ''' Initialize the local channel. KwArgs: - - userhome (string): (default='.') This is provided as a way to override and set a specific userhome - envs (dict) : A dictionary of env variables to be set when launching the shell ''' - self.userhome = os.path.abspath(".") envs = {} self.envs = envs local_env = os.environ.copy() @@ -52,7 +50,6 @@ def execute_wait(self, cmd, walltime=None, envs={}): cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, - cwd=self.userhome, env=current_env, shell=True, preexec_fn=os.setpgrp From d6f115e7c951c3b0735d77586477e89c3c5d5f0b Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 17:54:41 +0000 Subject: [PATCH 56/71] Rework environment handling in LocalChannel Environments are never passed to local channel, so the environment parameter is removed from execute_wait by this PR. This will change the environment behaviour of the local channel: commands will now receive a copy of the parent environment at time of command invocation. Prior to this PR, commands receive a copy of the parent environment at the time of channel initialization and would not see any changes made to the process environment after that. This new behaviour is more consistent with Popen. It is a user-facing change, though. Because it is user facing and not directly part of #3515 channel removal, it could be made into a PR now. --- parsl/channels/local/local.py | 20 ++----------------- .../tests/test_channels/test_local_channel.py | 19 ------------------ 2 files changed, 2 insertions(+), 37 deletions(-) diff --git a/parsl/channels/local/local.py b/parsl/channels/local/local.py index adb469e721..9f74ce16b9 100644 --- a/parsl/channels/local/local.py +++ b/parsl/channels/local/local.py @@ -1,4 +1,3 @@ -import copy import logging import os import subprocess @@ -15,42 +14,27 @@ class LocalChannel(RepresentationMixin): def __init__(self): ''' Initialize the local channel. - - KwArgs: - - envs (dict) : A dictionary of env variables to be set when launching the shell ''' - envs = {} - self.envs = envs - local_env = os.environ.copy() - self._envs = copy.deepcopy(local_env) - self._envs.update(envs) + pass - 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, this is not really used now. - 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 ) diff --git a/parsl/tests/test_channels/test_local_channel.py b/parsl/tests/test_channels/test_local_channel.py index e109cd841c..5f662af302 100644 --- a/parsl/tests/test_channels/test_local_channel.py +++ b/parsl/tests/test_channels/test_local_channel.py @@ -19,22 +19,3 @@ def test_env(): assert x, "HOME not found" print("RC:{} \nSTDOUT:{} \nSTDERR:{}".format(rc, stdout, stderr)) - - -@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" From 208c49d2448ec20230db8c093462411910dc2e4d Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 18:01:31 +0000 Subject: [PATCH 57/71] Correct docstring - this *is* used by the local provider. even though I think it is buggy. This can be merged now, not waiting till november. --- parsl/channels/local/local.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parsl/channels/local/local.py b/parsl/channels/local/local.py index 9f74ce16b9..ebd336540d 100644 --- a/parsl/channels/local/local.py +++ b/parsl/channels/local/local.py @@ -22,7 +22,7 @@ def execute_wait(self, cmd, walltime=None): Args: - cmd (string) : Commandline string to execute - - walltime (int) : walltime in seconds, this is not really used now. + - walltime (int) : walltime in seconds Returns: - retcode : Return code from the execution From fc85aac71bf0647858bf39064ca00407ad5a81fa Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 18:03:48 +0000 Subject: [PATCH 58/71] LocalChannel has no state to represent, so doesn't need a fancy repr() any more --- parsl/channels/local/local.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/parsl/channels/local/local.py b/parsl/channels/local/local.py index ebd336540d..49724f154e 100644 --- a/parsl/channels/local/local.py +++ b/parsl/channels/local/local.py @@ -2,12 +2,10 @@ import os import subprocess -from parsl.utils import RepresentationMixin - logger = logging.getLogger(__name__) -class LocalChannel(RepresentationMixin): +class LocalChannel: ''' This is not even really a channel, since opening a local shell is not heavy and done so infrequently that they do not need a persistent channel ''' From 84b86288dc534bd440b4480f3bad59aabc2f7ad4 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 18:04:42 +0000 Subject: [PATCH 59/71] Remove a now-irrelevant TODO in torque provider --- parsl/providers/torque/torque.py | 1 - 1 file changed, 1 deletion(-) diff --git a/parsl/providers/torque/torque.py b/parsl/providers/torque/torque.py index 48a6f4d0d4..6958c8348a 100644 --- a/parsl/providers/torque/torque.py +++ b/parsl/providers/torque/torque.py @@ -165,7 +165,6 @@ def submit(self, command, tasks_per_node, job_name="parsl.torque"): tasks_per_node) job_config = {} - # TODO : script_path might need to change to accommodate script dir set via channels job_config["submit_script_dir"] = self.script_dir job_config["nodes"] = self.nodes_per_block job_config["task_blocks"] = self.nodes_per_block * tasks_per_node From e74e084beab39929e5173c30e48d17445bd2656f Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 18:05:20 +0000 Subject: [PATCH 60/71] Remove local channel empty init --- parsl/channels/local/local.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/parsl/channels/local/local.py b/parsl/channels/local/local.py index 49724f154e..2d073ff14f 100644 --- a/parsl/channels/local/local.py +++ b/parsl/channels/local/local.py @@ -10,11 +10,6 @@ class LocalChannel: and done so infrequently that they do not need a persistent channel ''' - def __init__(self): - ''' Initialize the local channel. - ''' - pass - def execute_wait(self, cmd, walltime=None): ''' Synchronously execute a commandline string on the shell. From 63f94b4884c9a8e6ada7b50ebd83a98779404425 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 18:11:30 +0000 Subject: [PATCH 61/71] Get rid of LocalChannel empty class structure around execute_wait method-now-function --- parsl/channels/local/local.py | 67 +++++++++---------- parsl/providers/cluster_provider.py | 5 +- parsl/providers/local/local.py | 11 ++- .../tests/test_channels/test_large_output.py | 6 +- .../tests/test_channels/test_local_channel.py | 5 +- 5 files changed, 42 insertions(+), 52 deletions(-) diff --git a/parsl/channels/local/local.py b/parsl/channels/local/local.py index 2d073ff14f..feecfa0358 100644 --- a/parsl/channels/local/local.py +++ b/parsl/channels/local/local.py @@ -5,41 +5,36 @@ logger = logging.getLogger(__name__) -class LocalChannel: - ''' This is not even really a channel, since opening a local shell is not heavy - and done so infrequently that they do not need a persistent channel - ''' - - 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 +def execute_wait(cmd, walltime=None): + ''' Synchronously execute a commandline string on the shell. - Returns: - - retcode : Return code from the execution - - stdout : stdout string - - stderr : stderr string - ''' - try: - logger.debug("Creating process with command '%s'", cmd) - proc = subprocess.Popen( - cmd, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - shell=True, - preexec_fn=os.setpgrp - ) - logger.debug("Created process with pid %s. Performing communicate", proc.pid) - (stdout, stderr) = proc.communicate(timeout=walltime) - retcode = proc.returncode - logger.debug("Process %s returned %s", proc.pid, proc.returncode) + Args: + - cmd (string) : Commandline string to execute + - walltime (int) : walltime in seconds - except Exception: - logger.exception(f"Execution of command failed:\n{cmd}") - raise - else: - logger.debug("Execution of command in process %s completed normally", proc.pid) - - return (retcode, stdout.decode("utf-8"), stderr.decode("utf-8")) + Returns: + - retcode : Return code from the execution + - stdout : stdout string + - stderr : stderr string + ''' + try: + logger.debug("Creating process with command '%s'", cmd) + proc = subprocess.Popen( + cmd, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + shell=True, + preexec_fn=os.setpgrp + ) + logger.debug("Created process with pid %s. Performing communicate", proc.pid) + (stdout, stderr) = proc.communicate(timeout=walltime) + retcode = proc.returncode + logger.debug("Process %s returned %s", proc.pid, proc.returncode) + + except Exception: + logger.exception(f"Execution of command failed:\n{cmd}") + raise + else: + logger.debug("Execution of command in process %s completed normally", proc.pid) + + return (retcode, stdout.decode("utf-8"), stderr.decode("utf-8")) diff --git a/parsl/providers/cluster_provider.py b/parsl/providers/cluster_provider.py index ed74b97862..5e46e7fbfb 100644 --- a/parsl/providers/cluster_provider.py +++ b/parsl/providers/cluster_provider.py @@ -2,7 +2,7 @@ from abc import abstractmethod from string import Template -from parsl.channels.local.local import LocalChannel +from parsl.channels.local.local import execute_wait from parsl.launchers.base import Launcher from parsl.launchers.errors import BadLauncher from parsl.providers.base import ExecutionProvider @@ -53,7 +53,6 @@ def __init__(self, cmd_timeout=10): self._label = label - self.channel = LocalChannel() self.nodes_per_block = nodes_per_block self.init_blocks = init_blocks self.min_blocks = min_blocks @@ -74,7 +73,7 @@ def execute_wait(self, cmd, timeout=None): t = self.cmd_timeout if timeout is not None: t = timeout - return self.channel.execute_wait(cmd, t) + return execute_wait(cmd, t) def _write_submit_script(self, template, script_filename, job_name, configs): """Generate submit script and write it to a file. diff --git a/parsl/providers/local/local.py b/parsl/providers/local/local.py index 13b4899e53..bce7fa78db 100644 --- a/parsl/providers/local/local.py +++ b/parsl/providers/local/local.py @@ -2,7 +2,7 @@ import os import time -from parsl.channels.local.local import LocalChannel +from parsl.channels.local.local import execute_wait from parsl.jobs.states import JobState, JobStatus from parsl.launchers import SingleNodeLauncher from parsl.providers.base import ExecutionProvider @@ -45,7 +45,6 @@ def __init__(self, worker_init='', cmd_timeout=30, parallelism=1): - self.channel = LocalChannel() self._label = 'local' self.nodes_per_block = nodes_per_block self.launcher = launcher @@ -117,7 +116,7 @@ def status(self, job_ids): return [self.resources[jid]['status'] for jid in job_ids] def _is_alive(self, job_dict): - retcode, stdout, stderr = self.channel.execute_wait( + retcode, stdout, stderr = execute_wait( 'ps -p {} > /dev/null 2> /dev/null; echo "STATUS:$?" '.format( job_dict['remote_pid']), self.cmd_timeout) for line in stdout.split('\n'): @@ -222,11 +221,11 @@ def submit(self, command, tasks_per_node, job_name="parsl.localprovider"): # cancel the task later. # # We need to do the >/dev/null 2>&1 so that bash closes stdout, otherwise - # channel.execute_wait hangs reading the process stdout until all the + # execute_wait hangs reading the process stdout until all the # background commands complete. cmd = '/bin/bash -c \'echo - >{0}.ec && {{ {{ bash {0} 1>{0}.out 2>{0}.err ; ' \ 'echo $? > {0}.ec ; }} >/dev/null 2>&1 & echo "PID:$!" ; }}\''.format(script_path) - retcode, stdout, stderr = self.channel.execute_wait(cmd, self.cmd_timeout) + retcode, stdout, stderr = execute_wait(cmd, self.cmd_timeout) if retcode != 0: raise SubmitException(job_name, "Launch command exited with code {0}".format(retcode), stdout, stderr) @@ -257,7 +256,7 @@ def cancel(self, job_ids): job_dict['cancelled'] = True logger.debug("Terminating job/process ID: {0}".format(job)) cmd = "kill -- -$(ps -o pgid= {} | grep -o '[0-9]*')".format(job_dict['remote_pid']) - retcode, stdout, stderr = self.channel.execute_wait(cmd, self.cmd_timeout) + retcode, stdout, stderr = execute_wait(cmd, self.cmd_timeout) if retcode != 0: logger.warning("Failed to kill PID: {} and child processes on {}".format(job_dict['remote_pid'], self.label)) diff --git a/parsl/tests/test_channels/test_large_output.py b/parsl/tests/test_channels/test_large_output.py index bfc96f38bc..4c9b7e1021 100644 --- a/parsl/tests/test_channels/test_large_output.py +++ b/parsl/tests/test_channels/test_large_output.py @@ -1,6 +1,6 @@ import pytest -from parsl.channels.local.local import LocalChannel +from parsl.channels.local.local import execute_wait @pytest.mark.local @@ -11,10 +11,8 @@ def test_local_large_output_2210(): pipes filling up. """ - c = LocalChannel() - # this will output 128kb of stdout - c.execute_wait("yes | dd count=128 bs=1024", walltime=60) + execute_wait("yes | dd count=128 bs=1024", walltime=60) # if this test fails, execute_wait should raise a timeout # exception. diff --git a/parsl/tests/test_channels/test_local_channel.py b/parsl/tests/test_channels/test_local_channel.py index 5f662af302..0c9190e855 100644 --- a/parsl/tests/test_channels/test_local_channel.py +++ b/parsl/tests/test_channels/test_local_channel.py @@ -1,6 +1,6 @@ import pytest -from parsl.channels.local.local import LocalChannel +from parsl.channels.local.local import execute_wait @pytest.mark.local @@ -8,8 +8,7 @@ def test_env(): ''' Regression testing for issue #27 ''' - lc = LocalChannel() - rc, stdout, stderr = lc.execute_wait("env", 1) + rc, stdout, stderr = execute_wait("env", 1) stdout = stdout.split('\n') x = [s for s in stdout if s.startswith("PATH=")] From 4fdf78194256136ff1ad48f73349ccbcf1fcf7c2 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 24 Oct 2024 18:24:15 +0000 Subject: [PATCH 62/71] Move execute_wait into parsl.utils and remove parsl.channels remnant --- parsl/channels/__init__.py | 0 parsl/channels/local/__init__.py | 0 parsl/channels/local/local.py | 40 ------------------- parsl/providers/cluster_provider.py | 2 +- parsl/providers/local/local.py | 3 +- .../tests/test_channels/test_large_output.py | 2 +- .../tests/test_channels/test_local_channel.py | 2 +- parsl/utils.py | 35 ++++++++++++++++ 8 files changed, 39 insertions(+), 45 deletions(-) delete mode 100644 parsl/channels/__init__.py delete mode 100644 parsl/channels/local/__init__.py delete mode 100644 parsl/channels/local/local.py diff --git a/parsl/channels/__init__.py b/parsl/channels/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/parsl/channels/local/__init__.py b/parsl/channels/local/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/parsl/channels/local/local.py b/parsl/channels/local/local.py deleted file mode 100644 index feecfa0358..0000000000 --- a/parsl/channels/local/local.py +++ /dev/null @@ -1,40 +0,0 @@ -import logging -import os -import subprocess - -logger = logging.getLogger(__name__) - - -def execute_wait(cmd, walltime=None): - ''' Synchronously execute a commandline string on the shell. - - Args: - - cmd (string) : Commandline string to execute - - walltime (int) : walltime in seconds - - Returns: - - retcode : Return code from the execution - - stdout : stdout string - - stderr : stderr string - ''' - try: - logger.debug("Creating process with command '%s'", cmd) - proc = subprocess.Popen( - cmd, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - shell=True, - preexec_fn=os.setpgrp - ) - logger.debug("Created process with pid %s. Performing communicate", proc.pid) - (stdout, stderr) = proc.communicate(timeout=walltime) - retcode = proc.returncode - logger.debug("Process %s returned %s", proc.pid, proc.returncode) - - except Exception: - logger.exception(f"Execution of command failed:\n{cmd}") - raise - else: - logger.debug("Execution of command in process %s completed normally", proc.pid) - - return (retcode, stdout.decode("utf-8"), stderr.decode("utf-8")) diff --git a/parsl/providers/cluster_provider.py b/parsl/providers/cluster_provider.py index 5e46e7fbfb..db7ef9eaac 100644 --- a/parsl/providers/cluster_provider.py +++ b/parsl/providers/cluster_provider.py @@ -2,11 +2,11 @@ from abc import abstractmethod from string import Template -from parsl.channels.local.local import execute_wait from parsl.launchers.base import Launcher from parsl.launchers.errors import BadLauncher from parsl.providers.base import ExecutionProvider from parsl.providers.errors import SchedulerMissingArgs, ScriptPathError +from parsl.utils import execute_wait logger = logging.getLogger(__name__) diff --git a/parsl/providers/local/local.py b/parsl/providers/local/local.py index bce7fa78db..55994c31c3 100644 --- a/parsl/providers/local/local.py +++ b/parsl/providers/local/local.py @@ -2,7 +2,6 @@ import os import time -from parsl.channels.local.local import execute_wait from parsl.jobs.states import JobState, JobStatus from parsl.launchers import SingleNodeLauncher from parsl.providers.base import ExecutionProvider @@ -11,7 +10,7 @@ ScriptPathError, SubmitException, ) -from parsl.utils import RepresentationMixin +from parsl.utils import RepresentationMixin, execute_wait logger = logging.getLogger(__name__) diff --git a/parsl/tests/test_channels/test_large_output.py b/parsl/tests/test_channels/test_large_output.py index 4c9b7e1021..0558e600c7 100644 --- a/parsl/tests/test_channels/test_large_output.py +++ b/parsl/tests/test_channels/test_large_output.py @@ -1,6 +1,6 @@ import pytest -from parsl.channels.local.local import execute_wait +from parsl.utils import execute_wait @pytest.mark.local diff --git a/parsl/tests/test_channels/test_local_channel.py b/parsl/tests/test_channels/test_local_channel.py index 0c9190e855..6564bacb22 100644 --- a/parsl/tests/test_channels/test_local_channel.py +++ b/parsl/tests/test_channels/test_local_channel.py @@ -1,6 +1,6 @@ import pytest -from parsl.channels.local.local import execute_wait +from parsl.utils import execute_wait @pytest.mark.local diff --git a/parsl/utils.py b/parsl/utils.py index 0ea5d7d9eb..681b677b45 100644 --- a/parsl/utils.py +++ b/parsl/utils.py @@ -458,3 +458,38 @@ def sanitize_dns_subdomain_rfc1123(raw_string: str) -> str: raise ValueError(f"Sanitized DNS subdomain is empty for input '{raw_string}'") return sanitized + + +def execute_wait(cmd: str, walltime: Union[float, int, None] = None) -> Tuple[int, str, str]: + ''' Synchronously execute a commandline string on the shell. + + Args: + - cmd (string) : Commandline string to execute + - walltime (int) : walltime in seconds + + Returns: + - retcode : Return code from the execution + - stdout : stdout string + - stderr : stderr string + ''' + try: + logger.debug("Creating process with command '%s'", cmd) + proc = subprocess.Popen( + cmd, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + shell=True, + preexec_fn=os.setpgrp + ) + logger.debug("Created process with pid %s. Performing communicate", proc.pid) + (stdout, stderr) = proc.communicate(timeout=walltime) + retcode = proc.returncode + logger.debug("Process %s returned %s", proc.pid, proc.returncode) + + except Exception: + logger.exception(f"Execution of command failed:\n{cmd}") + raise + else: + logger.debug("Execution of command in process %s completed normally", proc.pid) + + return (retcode, stdout.decode("utf-8"), stderr.decode("utf-8")) From ab8ade3774359226c78f0e0ccbfd7cf90294ef7a Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Fri, 1 Nov 2024 11:37:15 +0000 Subject: [PATCH 63/71] fix up new test that should not use channels --- .../test_scaling/test_worker_interchange_bad_messages_3262.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/parsl/tests/test_scaling/test_worker_interchange_bad_messages_3262.py b/parsl/tests/test_scaling/test_worker_interchange_bad_messages_3262.py index eee128634e..414f67cab6 100644 --- a/parsl/tests/test_scaling/test_worker_interchange_bad_messages_3262.py +++ b/parsl/tests/test_scaling/test_worker_interchange_bad_messages_3262.py @@ -6,7 +6,6 @@ import zmq import parsl -from parsl.channels import LocalChannel from parsl.config import Config from parsl.executors import HighThroughputExecutor from parsl.launchers import SimpleLauncher @@ -24,7 +23,6 @@ def fresh_config(): cores_per_worker=1, encrypted=False, provider=LocalProvider( - channel=LocalChannel(), init_blocks=0, min_blocks=0, max_blocks=0, From 22d6799c8c08dc2d344b28a38d4c250c2f312681 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 7 Nov 2024 17:43:39 +0000 Subject: [PATCH 64/71] fix tests broken by removal of provider.channel --- parsl/tests/test_providers/test_pbspro_template.py | 1 - parsl/tests/test_providers/test_slurm_template.py | 1 - 2 files changed, 2 deletions(-) diff --git a/parsl/tests/test_providers/test_pbspro_template.py b/parsl/tests/test_providers/test_pbspro_template.py index ed8ef8e4a2..1264731120 100644 --- a/parsl/tests/test_providers/test_pbspro_template.py +++ b/parsl/tests/test_providers/test_pbspro_template.py @@ -14,7 +14,6 @@ def test_submit_script_basic(tmp_path): queue="debug" ) provider.script_dir = tmp_path - provider.channel.script_dir = tmp_path job_id = str(random.randint(55000, 59000)) provider.execute_wait = mock.Mock(spec=PBSProProvider.execute_wait) provider.execute_wait.return_value = (0, job_id, "") diff --git a/parsl/tests/test_providers/test_slurm_template.py b/parsl/tests/test_providers/test_slurm_template.py index 5c36026bfb..55074fefe3 100644 --- a/parsl/tests/test_providers/test_slurm_template.py +++ b/parsl/tests/test_providers/test_slurm_template.py @@ -15,7 +15,6 @@ def test_submit_script_basic(tmp_path): partition="debug" ) provider.script_dir = tmp_path - provider.channel.script_dir = tmp_path job_id = str(random.randint(55000, 59000)) provider.execute_wait = mock.MagicMock(spec=SlurmProvider.execute_wait) provider.execute_wait.return_value = (0, f"Submitted batch job {job_id}", "") From 20b42fed6996cdfdf169e4c4a9c839679e9fa4d5 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Fri, 8 Nov 2024 10:17:10 +0000 Subject: [PATCH 65/71] Remove channel push/pull file --- parsl/channels/base.py | 28 --------------------------- parsl/channels/local/local.py | 36 ----------------------------------- 2 files changed, 64 deletions(-) diff --git a/parsl/channels/base.py b/parsl/channels/base.py index e8acfc1088..96fa0b2582 100644 --- a/parsl/channels/base.py +++ b/parsl/channels/base.py @@ -53,34 +53,6 @@ def script_dir(self) -> str: def script_dir(self, value: str) -> None: pass - @abstractmethod - def push_file(self, source: str, dest_dir: str) -> str: - ''' Channel will take care of moving the file from source to the destination - directory - - Args: - source (string) : Full filepath of the file to be moved - dest_dir (string) : Absolute path of the directory to move to - - Returns: - destination_path (string) - ''' - pass - - @abstractmethod - def pull_file(self, remote_source: str, local_dir: str) -> str: - ''' Transport file on the remote side to a local directory - - Args: - remote_source (string): remote_source - local_dir (string): Local directory to copy to - - - Returns: - destination_path (string) - ''' - pass - @abstractmethod def makedirs(self, path: str, mode: int = 0o511, exist_ok: bool = False) -> None: """Create a directory. diff --git a/parsl/channels/local/local.py b/parsl/channels/local/local.py index 40b7eac34f..58bd6af849 100644 --- a/parsl/channels/local/local.py +++ b/parsl/channels/local/local.py @@ -1,10 +1,8 @@ import logging import os -import shutil import subprocess from parsl.channels.base import Channel -from parsl.channels.errors import FileCopyException from parsl.utils import RepresentationMixin logger = logging.getLogger(__name__) @@ -57,40 +55,6 @@ def execute_wait(self, cmd, walltime=None): return (retcode, stdout.decode("utf-8"), stderr.decode("utf-8")) - def push_file(self, source, dest_dir): - ''' If the source files dirpath is the same as dest_dir, a copy - is not necessary, and nothing is done. Else a copy is made. - - Args: - - source (string) : Path to the source file - - dest_dir (string) : Path to the directory to which the files is to be copied - - Returns: - - destination_path (String) : Absolute path of the destination file - - Raises: - - FileCopyException : If file copy failed. - ''' - - local_dest = os.path.join(dest_dir, os.path.basename(source)) - - # Only attempt to copy if the target dir and source dir are different - if os.path.dirname(source) != dest_dir: - try: - shutil.copyfile(source, local_dest) - os.chmod(local_dest, 0o700) - - except OSError as e: - raise FileCopyException(e, "localhost") - - else: - os.chmod(local_dest, 0o700) - - return local_dest - - def pull_file(self, remote_source, local_dir): - return self.push_file(remote_source, local_dir) - def isdir(self, path): """Return true if the path refers to an existing directory. From 6a9666e143a6b8093054877e56c522a0d24677b7 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Fri, 8 Nov 2024 10:20:10 +0000 Subject: [PATCH 66/71] Remove channel push_file support from SlurmProvider This has been a basically dead code path since removal of non-remote channels: Either a user chooses to not move files (overriding move_files to False) and the push_file was not used. Or the default push_file was used, but: * this is always LocalChannel now (PR #3677) * the script directory is always the local script directory (PR #3688) and so the local channel code always skips making a copy This commit simplies all of that away into using the script path directly with putting it through the above complicated no-op. --- parsl/providers/slurm/slurm.py | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/parsl/providers/slurm/slurm.py b/parsl/providers/slurm/slurm.py index 2aa855bb48..865ca6a52d 100644 --- a/parsl/providers/slurm/slurm.py +++ b/parsl/providers/slurm/slurm.py @@ -107,7 +107,6 @@ class SlurmProvider(ClusterProvider, RepresentationMixin): :class:`~parsl.launchers.SingleNodeLauncher` (the default), :class:`~parsl.launchers.SrunLauncher`, or :class:`~parsl.launchers.AprunLauncher` - move_files : Optional[Bool]: should files be moved? by default, Parsl will try to move files. """ @typeguard.typechecked @@ -130,7 +129,6 @@ def __init__(self, worker_init: str = '', cmd_timeout: int = 10, exclusive: bool = True, - move_files: bool = True, launcher: Launcher = SingleNodeLauncher()): label = 'slurm' super().__init__(label, @@ -148,7 +146,6 @@ def __init__(self, self.cores_per_node = cores_per_node self.mem_per_node = mem_per_node self.exclusive = exclusive - self.move_files = move_files self.account = account self.qos = qos self.constraint = constraint @@ -293,14 +290,7 @@ def submit(self, command: str, tasks_per_node: int, job_name="parsl.slurm") -> s logger.debug("Writing submit script") self._write_submit_script(template_string, script_path, job_name, job_config) - if self.move_files: - logger.debug("moving files") - channel_script_path = self.channel.push_file(script_path, self.channel.script_dir) - else: - logger.debug("not moving files") - channel_script_path = script_path - - retcode, stdout, stderr = self.execute_wait("sbatch {0}".format(channel_script_path)) + retcode, stdout, stderr = self.execute_wait("sbatch {0}".format(script_path)) if retcode == 0: for line in stdout.split('\n'): From 38d29e14b9e15f4c911c32116f02b362bd581117 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Fri, 8 Nov 2024 10:41:46 +0000 Subject: [PATCH 67/71] Remove push file from pbspro. see slurm commit for justification. --- parsl/providers/pbspro/pbspro.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/parsl/providers/pbspro/pbspro.py b/parsl/providers/pbspro/pbspro.py index 752f504334..71c958f000 100644 --- a/parsl/providers/pbspro/pbspro.py +++ b/parsl/providers/pbspro/pbspro.py @@ -183,15 +183,13 @@ def submit(self, command, tasks_per_node, job_name="parsl"): logger.debug("Writing submit script") self._write_submit_script(self.template_string, script_path, job_name, job_config) - channel_script_path = self.channel.push_file(script_path, self.channel.script_dir) - submit_options = '' if self.queue is not None: submit_options = '{0} -q {1}'.format(submit_options, self.queue) if self.account is not None: submit_options = '{0} -A {1}'.format(submit_options, self.account) - launch_cmd = "qsub {0} {1}".format(submit_options, channel_script_path) + launch_cmd = "qsub {0} {1}".format(submit_options, script_path) retcode, stdout, stderr = self.execute_wait(launch_cmd) job_id = None From 85031e34c1861f37b6712e4b0d36f9234341f529 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Fri, 8 Nov 2024 11:06:57 +0000 Subject: [PATCH 68/71] remove push/pull/move from LocalProvider --- parsl/providers/local/local.py | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/parsl/providers/local/local.py b/parsl/providers/local/local.py index f13521466a..5ecf174df2 100644 --- a/parsl/providers/local/local.py +++ b/parsl/providers/local/local.py @@ -32,9 +32,6 @@ class LocalProvider(ExecutionProvider, RepresentationMixin): Ratio of provisioned task slots to active tasks. A parallelism value of 1 represents aggressive scaling where as many resources as possible are used; parallelism close to 0 represents the opposite situation in which as few resources as possible (i.e., min_blocks) are used. - move_files : Optional[Bool] - Should files be moved? By default, Parsl will try to figure this out itself (= None). - If True, then will always move. If False, will never move. worker_init : str Command to be run before starting a worker, such as 'module load Anaconda; source activate env'. """ @@ -48,8 +45,7 @@ def __init__(self, max_blocks=1, worker_init='', cmd_timeout=30, - parallelism=1, - move_files=None): + parallelism=1): self.channel = channel self._label = 'local' self.nodes_per_block = nodes_per_block @@ -61,7 +57,6 @@ def __init__(self, self.parallelism = parallelism self.script_dir = None self.cmd_timeout = cmd_timeout - self.move_files = move_files # Dictionary that keeps track of jobs, keyed on job_id self.resources = {} @@ -83,7 +78,6 @@ def status(self, job_ids): if job_dict['status'] and job_dict['status'].terminal: # We already checked this and it can't change after that continue - # Script path should point to remote path if _should_move_files() is True script_path = job_dict['script_path'] alive = self._is_alive(job_dict) @@ -137,8 +131,6 @@ def _is_alive(self, job_dict): def _job_file_path(self, script_path: str, suffix: str) -> str: path = '{0}{1}'.format(script_path, suffix) - if self._should_move_files(): - path = self.channel.pull_file(path, self.script_dir) return path def _read_job_file(self, script_path: str, suffix: str) -> str: @@ -216,9 +208,6 @@ def submit(self, command, tasks_per_node, job_name="parsl.localprovider"): job_id = None remote_pid = None - if self._should_move_files(): - logger.debug("Pushing start script") - script_path = self.channel.push_file(script_path, self.channel.script_dir) logger.debug("Launching") # We need to capture the exit code and the streams, so we put them in files. We also write @@ -254,9 +243,6 @@ def submit(self, command, tasks_per_node, job_name="parsl.localprovider"): return job_id - def _should_move_files(self): - return (self.move_files is None and not isinstance(self.channel, LocalChannel)) or (self.move_files) - def cancel(self, job_ids): ''' Cancels the jobs specified by a list of job ids From 74ed1a52f73964ff6232f73823b5bed6eb79cfac Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Fri, 8 Nov 2024 11:15:22 +0000 Subject: [PATCH 69/71] do all the other providers, basically same changes as slurm --- parsl/providers/condor/condor.py | 8 +++----- parsl/providers/grid_engine/grid_engine.py | 5 ++--- parsl/providers/lsf/lsf.py | 14 ++------------ parsl/providers/torque/torque.py | 4 +--- 4 files changed, 8 insertions(+), 23 deletions(-) diff --git a/parsl/providers/condor/condor.py b/parsl/providers/condor/condor.py index a736386d38..c8142c4026 100644 --- a/parsl/providers/condor/condor.py +++ b/parsl/providers/condor/condor.py @@ -245,16 +245,14 @@ def submit(self, command, tasks_per_node, job_name="parsl.condor"): with open(userscript_path, 'w') as f: f.write(job_config["worker_init"] + '\n' + wrapped_command) - user_script_path = self.channel.push_file(userscript_path, self.channel.script_dir) - the_input_files = [user_script_path] + self.transfer_input_files + the_input_files = [userscript_path] + self.transfer_input_files job_config["input_files"] = ','.join(the_input_files) - job_config["job_script"] = os.path.basename(user_script_path) + job_config["job_script"] = os.path.basename(userscript_path) # Construct and move the submit script self._write_submit_script(template_string, script_path, job_name, job_config) - channel_script_path = self.channel.push_file(script_path, self.channel.script_dir) - cmd = "condor_submit {0}".format(channel_script_path) + cmd = "condor_submit {0}".format(script_path) try: retcode, stdout, stderr = self.execute_wait(cmd) except Exception as e: diff --git a/parsl/providers/grid_engine/grid_engine.py b/parsl/providers/grid_engine/grid_engine.py index e7db987022..ddedcaa3e8 100644 --- a/parsl/providers/grid_engine/grid_engine.py +++ b/parsl/providers/grid_engine/grid_engine.py @@ -142,11 +142,10 @@ def submit(self, command, tasks_per_node, job_name="parsl.sge"): logger.debug("Writing submit script") self._write_submit_script(template_string, script_path, job_name, job_config) - channel_script_path = self.channel.push_file(script_path, self.channel.script_dir) if self.queue is not None: - cmd = "qsub -q {0} -terse {1}".format(self.queue, channel_script_path) + cmd = "qsub -q {0} -terse {1}".format(self.queue, script_path) else: - cmd = "qsub -terse {0}".format(channel_script_path) + cmd = "qsub -terse {0}".format(script_path) retcode, stdout, stderr = self.execute_wait(cmd) if retcode == 0: diff --git a/parsl/providers/lsf/lsf.py b/parsl/providers/lsf/lsf.py index 8f18f5c879..b446b063a4 100644 --- a/parsl/providers/lsf/lsf.py +++ b/parsl/providers/lsf/lsf.py @@ -68,7 +68,6 @@ class LSFProvider(ClusterProvider, RepresentationMixin): :class:`~parsl.launchers.SingleNodeLauncher` (the default), :class:`~parsl.launchers.SrunLauncher`, or :class:`~parsl.launchers.AprunLauncher` - move_files : Optional[Bool]: should files be moved? by default, Parsl will try to move files. bsub_redirection: Bool Should a redirection symbol "<" be included when submitting jobs, i.e., Bsub < job_script. request_by_nodes: Bool @@ -92,7 +91,6 @@ def __init__(self, project=None, queue=None, cmd_timeout=120, - move_files=True, bsub_redirection=False, request_by_nodes=True, launcher=SingleNodeLauncher()): @@ -112,7 +110,6 @@ def __init__(self, self.queue = queue self.cores_per_block = cores_per_block self.cores_per_node = cores_per_node - self.move_files = move_files self.bsub_redirection = bsub_redirection self.request_by_nodes = request_by_nodes @@ -230,17 +227,10 @@ def submit(self, command, tasks_per_node, job_name="parsl.lsf"): logger.debug("Writing submit script") self._write_submit_script(template_string, script_path, job_name, job_config) - if self.move_files: - logger.debug("moving files") - channel_script_path = self.channel.push_file(script_path, self.channel.script_dir) - else: - logger.debug("not moving files") - channel_script_path = script_path - if self.bsub_redirection: - cmd = "bsub < {0}".format(channel_script_path) + cmd = "bsub < {0}".format(script_path) else: - cmd = "bsub {0}".format(channel_script_path) + cmd = "bsub {0}".format(script_path) retcode, stdout, stderr = super().execute_wait(cmd) job_id = None diff --git a/parsl/providers/torque/torque.py b/parsl/providers/torque/torque.py index c15591706c..7992893abb 100644 --- a/parsl/providers/torque/torque.py +++ b/parsl/providers/torque/torque.py @@ -189,15 +189,13 @@ def submit(self, command, tasks_per_node, job_name="parsl.torque"): logger.debug("Writing submit script") self._write_submit_script(self.template_string, script_path, job_name, job_config) - channel_script_path = self.channel.push_file(script_path, self.channel.script_dir) - submit_options = '' if self.queue is not None: submit_options = '{0} -q {1}'.format(submit_options, self.queue) if self.account is not None: submit_options = '{0} -A {1}'.format(submit_options, self.account) - launch_cmd = "qsub {0} {1}".format(submit_options, channel_script_path) + launch_cmd = "qsub {0} {1}".format(submit_options, script_path) retcode, stdout, stderr = self.execute_wait(launch_cmd) job_id = None From 5c566ca9b9ceb722cd039bd5902e193765a4b48e Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Tue, 3 Dec 2024 19:51:57 +0000 Subject: [PATCH 70/71] restore type signature --- parsl/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parsl/utils.py b/parsl/utils.py index 681b677b45..b6544d63d2 100644 --- a/parsl/utils.py +++ b/parsl/utils.py @@ -460,7 +460,7 @@ def sanitize_dns_subdomain_rfc1123(raw_string: str) -> str: return sanitized -def execute_wait(cmd: str, walltime: Union[float, int, None] = None) -> Tuple[int, str, str]: +def execute_wait(cmd: str, walltime: Optional[int] = None) -> Tuple[int, str, str]: ''' Synchronously execute a commandline string on the shell. Args: From c162b1c8bb58d6f9032b078e2da939c664a93278 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Tue, 3 Dec 2024 19:52:20 +0000 Subject: [PATCH 71/71] Revert "Remove unused channel test files" This reverts commit 8fbd20f01592a8e149435d02b54e5e774ca1bfc5. --- parsl/tests/integration/test_channels/__init__.py | 0 parsl/tests/integration/test_channels/remote_run.sh | 5 +++++ 2 files changed, 5 insertions(+) create mode 100644 parsl/tests/integration/test_channels/__init__.py create mode 100644 parsl/tests/integration/test_channels/remote_run.sh diff --git a/parsl/tests/integration/test_channels/__init__.py b/parsl/tests/integration/test_channels/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/parsl/tests/integration/test_channels/remote_run.sh b/parsl/tests/integration/test_channels/remote_run.sh new file mode 100644 index 0000000000..aa4945d859 --- /dev/null +++ b/parsl/tests/integration/test_channels/remote_run.sh @@ -0,0 +1,5 @@ +#!/bin/bash +echo "Hostname: $HOSTNAME" +echo "Cpu info -----" +cat /proc/cpuinfo +echo "Done----------"