Fix platform subshell evaluating more than once for tasks triggered in paused workflow

cylc/flow/exceptions.py

@@ -184,13 +184,13 @@ class PlatformError(CylcError):
     def __init__(
         self,
         message: str,
-        platform_name: str,
+        platform_name: str | None,
         *,
-        ctx: 'Optional[SubFuncContext]' = None,
-        cmd: Union[str, Sequence[str], None] = None,
-        ret_code: Optional[int] = None,
-        out: Optional[str] = None,
-        err: Optional[str] = None
+        ctx: 'SubFuncContext | None' = None,
+        cmd: str | Sequence[str] | None = None,
+        ret_code: int | None = None,
+        out: str | None = None,
+        err: str | None = None
     ) -> None:
         self.msg = message
         self.platform_name = platform_name

cylc/flow/platforms.py

@@ -128,7 +128,7 @@ def get_platform(
 
     Returns:
         platform: A platform definition dictionary. Uses either
-            get_platform() or platform_name_from_job_info(), but to the
+            get_platform() or _platform_name_from_job_info(), but to the
             user these look the same. This will be None if the platform
             definition uses a subshell.
 
@@ -169,7 +169,7 @@ def get_platform(
     task_job_section = task_conf['job'] if 'job' in task_conf else {}
     task_remote_section = task_conf['remote'] if 'remote' in task_conf else {}
     return platform_from_name(
-        platform_name_from_job_info(
+        _platform_name_from_job_info(
             glbl_cfg().get(['platforms']),
             task_job_section,
             task_remote_section,
@@ -329,7 +329,7 @@ def get_platform_from_group(
         return HOST_SELECTION_METHODS[method](platform_names)
 
 
-def platform_name_from_job_info(
+def _platform_name_from_job_info(
     platforms: Union[dict, 'OrderedDictWithDefaults'],
     job: Dict[str, Any],
     remote: Dict[str, Any],
@@ -407,14 +407,14 @@ def platform_name_from_job_info(
         ... }
         >>> job = {'batch system': 'slurm'}
         >>> remote = {'host': 'localhost'}
-        >>> platform_name_from_job_info(platforms, job, remote)
+        >>> _platform_name_from_job_info(platforms, job, remote)
         'sugar'
         >>> remote = {}
-        >>> platform_name_from_job_info(platforms, job, remote)
+        >>> _platform_name_from_job_info(platforms, job, remote)
         'sugar'
         >>> remote ={'host': 'desktop92'}
         >>> job = {}
-        >>> platform_name_from_job_info(platforms, job, remote)
+        >>> _platform_name_from_job_info(platforms, job, remote)
         'desktop92'
     """
 
@@ -581,9 +581,9 @@ def fail_if_platform_and_host_conflict(
     if 'platform' in task_conf and task_conf['platform']:
         fail_items = [
             f'\n * [{section}]{key}'
-            for section, keys in FORBIDDEN_WITH_PLATFORM.items()
+            for section, settings in FORBIDDEN_WITH_PLATFORM.items()
             if section in task_conf
-            for key, _ in keys.items()
+            for key in settings.keys()
             if (
                 key in task_conf[section] and
                 task_conf[section][key] is not None

cylc/flow/task_job_mgr.py

@@ -56,7 +56,6 @@
     NoPlatformsError,
     PlatformError,
     PlatformLookupError,
-    WorkflowConfigError,
 )
 from cylc.flow.hostuserutil import (
     get_host,
@@ -70,6 +69,7 @@
 from cylc.flow.pathutil import get_remote_workflow_run_job_dir
 from cylc.flow.platforms import (
     FORBIDDEN_WITH_PLATFORM,
+    fail_if_platform_and_host_conflict,
     get_host_from_platform,
     get_install_target_from_platform,
     get_localhost_install_target,
@@ -1166,15 +1166,7 @@ def _prep_submit_task_job(
         # - host exists - eval host_n
         # remove at:
         #     Cylc8.x
-        if (
-            rtconfig['platform'] is not None and
-            rtconfig['remote']['host'] is not None
-        ):
-            raise WorkflowConfigError(
-                "A mixture of Cylc 7 (host) and Cylc 8 (platform) "
-                "logic should not be used. In this case for the task "
-                f"\"{itask.identity}\" the following are not compatible:\n"
-            )
+        fail_if_platform_and_host_conflict(rtconfig, itask.tdef.name)
 
         host_name, platform_name = None, None
         try:
@@ -1204,8 +1196,8 @@ def _prep_submit_task_job(
             )
             return False
         else:
-            # host/platform select not ready
             if host_name is None and platform_name is None:
+                # host/platform select not ready
                 return None
             elif (
                 host_name is None

cylc/flow/task_pool.py

@@ -901,8 +901,7 @@ def remove(self, itask: 'TaskProxy', reason: Optional[str] = None) -> None:
         except KeyError:
             pass
         else:
-            with suppress(KeyError):
-                self.tasks_to_trigger_now.remove(itask)
+            self.tasks_to_trigger_now.discard(itask)
             self.tasks_removed = True
             self.active_tasks_changed = True
             if not self.active_tasks[itask.point]:
@@ -1043,7 +1042,7 @@ def count_active_tasks(self):
 
         return active_task_counter, pre_prep_tasks
 
-    def release_queued_tasks(self):
+    def release_queued_tasks(self) -> set['TaskProxy']:
         """Return list of queue-released tasks awaiting job prep.
 
         Note:
@@ -1072,7 +1071,7 @@ def release_queued_tasks(self):
                 self.spawn_on_all_outputs(itask)
 
         # Note: released and pre_prep_tasks can overlap
-        return list(set(released + pre_prep_tasks))
+        return set(released + pre_prep_tasks)
 
     def get_min_point(self):
         """Return the minimum cycle point currently in the pool."""

cylc/flow/task_remote_mgr.py

@@ -37,7 +37,6 @@
     Dict,
     List,
     NamedTuple,
-    Optional,
     Set,
     TYPE_CHECKING,
     Tuple,
@@ -120,8 +119,8 @@ def __init__(self, workflow, proc_pool, bad_hosts, db_mgr, server):
         self.server: WorkflowRuntimeServer = server
 
     def _subshell_eval(
-        self, eval_str: str, command_pattern: re.Pattern
-    ) -> Optional[str]:
+        self, eval_str: str | None, command_pattern: re.Pattern
+    ) -> str | None:
         """Evaluate a platform or host from a possible subshell string.
 
         Arguments:
@@ -175,7 +174,7 @@ def _subshell_eval(
     # BACK COMPAT: references to "host"
         # remove at:
         #     Cylc8.x
-    def eval_host(self, host_str: str) -> Optional[str]:
+    def eval_host(self, host_str: str | None) -> str | None:
         """Evaluate a host from a possible subshell string.
 
         Args:
@@ -191,7 +190,7 @@ def eval_host(self, host_str: str) -> Optional[str]:
             return 'localhost'
         return host
 
-    def eval_platform(self, platform_str: str) -> Optional[str]:
+    def eval_platform(self, platform_str: str | None) -> str | None:
         """Evaluate a platform from a possible subshell string.
 
         Args:

tests/unit/test_platforms.py

@@ -16,23 +16,31 @@
 #
 # Tests for the platform lookup.
 
+from typing import (
+    Any,
+    Dict,
+    List,
+    Optional,
+    Type,
+)
+
 import pytest
-from typing import Any, Dict, List, Optional, Type
 
+from cylc.flow.exceptions import (
+    GlobalConfigError,
+    PlatformLookupError,
+)
 from cylc.flow.parsec.OrderedDict import OrderedDictWithDefaults
 from cylc.flow.platforms import (
+    _platform_name_from_job_info,
+    _validate_single_host,
+    generic_items_match,
+    get_install_target_from_platform,
+    get_install_target_to_platforms_map,
     get_platform,
     get_platform_deprecated_settings,
     is_platform_definition_subshell,
-    platform_from_name, platform_name_from_job_info,
-    get_install_target_from_platform,
-    get_install_target_to_platforms_map,
-    generic_items_match,
-    _validate_single_host
-)
-from cylc.flow.exceptions import (
-    PlatformLookupError,
-    GlobalConfigError
+    platform_from_name,
 )
 from cylc.flow.run_modes import JOBLESS_MODES
 
@@ -270,11 +278,11 @@ def test_similar_but_not_exact_match():
     ]
 )
 def test_platform_name_from_job_info_basic(job, remote, returns):
-    assert platform_name_from_job_info(PLATFORMS, job, remote) == returns
+    assert _platform_name_from_job_info(PLATFORMS, job, remote) == returns
 
 
 def test_platform_name_from_job_info_evaluated_hostname():
-    result = platform_name_from_job_info(
+    result = _platform_name_from_job_info(
         PLATFORMS,
         {'batch system': 'background'},
         {'host': '$(cat tiddles)'},
@@ -294,7 +302,7 @@ def test_platform_name_from_job_info_ordered_dict_comparison():
     platform.defaults_['Made up key'] = {}
     platform.update(PLATFORMS['hpc1-bg'])
     platforms = {'hpc1-bg': platform, 'dobbie': PLATFORMS['sugar']}
-    assert platform_name_from_job_info(platforms, job, remote) == 'hpc1-bg'
+    assert _platform_name_from_job_info(platforms, job, remote) == 'hpc1-bg'
 
 
 # Cases where the error ought to be raised because no matching platform should
@@ -321,7 +329,7 @@ def test_platform_name_from_job_info_ordered_dict_comparison():
 )
 def test_reverse_PlatformLookupError(job, remote):
     with pytest.raises(PlatformLookupError):
-        platform_name_from_job_info(PLATFORMS, job, remote)
+        _platform_name_from_job_info(PLATFORMS, job, remote)
 
 
 # An example of a global config with two Spice systems available
@@ -359,7 +367,7 @@ def test_platform_name_from_job_info_two_spices(
         },
 
     }
-    assert platform_name_from_job_info(platforms, job, remote) == returns
+    assert _platform_name_from_job_info(platforms, job, remote) == returns
 
 
 # An example of two platforms with the same hosts and job runner settings
@@ -406,7 +414,7 @@ def test_platform_name_from_job_info_similar_platforms(
             'job runner': 'background'
         },
     }
-    assert platform_name_from_job_info(platforms, job, remote) == returns
+    assert _platform_name_from_job_info(platforms, job, remote) == returns
 
 
 # -----------------------------------------------------------------------------