From 0fe4ead52803f2f2d9c2533f46d995cf3d777609 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 7 Nov 2024 16:24:17 +0000 Subject: [PATCH 1/3] Move Channel plugin documentation to a removed section (#3686) ## Type of change - Update to human readable text: Documentation/error messages/comments --- docs/userguide/plugins.rst | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/docs/userguide/plugins.rst b/docs/userguide/plugins.rst index c3c38dea63..cd9244960c 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,10 @@ 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 deprecated ``Channel`` abstraction. See +`issue 3515 `_ +for further discussion on its removal. From b531ecc34fb8815bcf31183ba9406cb8a14b97ae Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 7 Nov 2024 16:39:32 +0000 Subject: [PATCH 2/3] Remove the opportunity to configure script_dir for LocalChannel (#3684) Prior to this PR, if the argument defaulted to None, then the DFK would set a script dir based on the workflow rundir. Post this PR, this default behaviour is the only behaviour and the user cannot specify an overriding value. Future PRs will remove channel level script directories entirely. This is part of issues #3515 removing channels, # Changed Behaviour This PR removes a little-used user configuration option. ## Type of change - Code maintenance/cleanup --- parsl/channels/local/local.py | 4 ++-- parsl/tests/test_providers/test_pbspro_template.py | 3 ++- parsl/tests/test_providers/test_slurm_template.py | 3 ++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/parsl/channels/local/local.py b/parsl/channels/local/local.py index ab301e1f19..9adf2f3fb3 100644 --- a/parsl/channels/local/local.py +++ b/parsl/channels/local/local.py @@ -15,14 +15,14 @@ class LocalChannel(Channel, RepresentationMixin): and done so infrequently that they do not need a persistent channel ''' - def __init__(self, script_dir=None): + def __init__(self): ''' Initialize the local channel. script_dir is required by set to a default. KwArgs: - script_dir (string): Directory to place scripts ''' self.hostname = "localhost" - self.script_dir = script_dir + self.script_dir = None def execute_wait(self, cmd, walltime=None): ''' 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 b0921956fcfc2dd5127dce6c3731a5d5458246b1 Mon Sep 17 00:00:00 2001 From: Ben Clifford Date: Thu, 7 Nov 2024 16:39:43 +0000 Subject: [PATCH 3/3] Remove channel closing at shutdown (#3685) LocalChannel, the only extant channel, does not have any close() behaviour. This is part of #3515 channel removal. ## Type of change - Code maintenance/cleanup --- parsl/channels/base.py | 6 ----- parsl/channels/local/local.py | 5 ---- parsl/dataflow/dflow.py | 11 --------- parsl/tests/test_channels/test_dfk_close.py | 26 --------------------- 4 files changed, 48 deletions(-) delete mode 100644 parsl/tests/test_channels/test_dfk_close.py diff --git a/parsl/channels/base.py b/parsl/channels/base.py index 001482a6e8..e8acfc1088 100644 --- a/parsl/channels/base.py +++ b/parsl/channels/base.py @@ -81,12 +81,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 9adf2f3fb3..d06d40549a 100644 --- a/parsl/channels/local/local.py +++ b/parsl/channels/local/local.py @@ -92,11 +92,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 4b45e2cd89..d6cb0597fc 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 for {executor.label}") - - assert hasattr(executor.provider, 'channel'), "Provider with .script_dir 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 for {executor.label}") - logger.info("Terminated executors") self.time_completed = datetime.datetime.now() 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