From d56e9dd772f7a21ff4f49802903c8293b8deb25d Mon Sep 17 00:00:00 2001
From: Tim Pillinger <26465611+wxtim@users.noreply.github.com>
Date: Tue, 11 Apr 2023 11:03:16 +0100
Subject: [PATCH] response to review

---
 cylc/flow/exceptions.py      | 16 ++++++++++++----
 cylc/flow/platforms.py       |  2 +-
 cylc/flow/task_remote_mgr.py | 20 +++++++++++---------
 3 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/cylc/flow/exceptions.py b/cylc/flow/exceptions.py
index 94d90f25066..d017193b6bd 100644
--- a/cylc/flow/exceptions.py
+++ b/cylc/flow/exceptions.py
@@ -436,12 +436,20 @@ def __str__(self):
 
 
 class NoPlatformsError(CylcError):
-    """None of the platforms of a given group were reachable."""
-    def __init__(self, platform_group):
-        self.platform_group = platform_group
+    """None of the platforms of a given set were reachable.
+
+    Instatiation args:
+        identity: The name of the platform group or install target
+        _type: Whether the set of platforms is a platform group or an
+            install target
+    """
+    def __init__(self, identity: str, set_type: str = 'group'):
+        self.identity = identity
+        self.type = set_type
 
     def __str__(self):
-        return f'Unable to find a platform from group {self.platform_group}.'
+        return (
+            f'Unable to find a platform from {self.type} {self.identity}.')
 
 
 class CylcVersionError(CylcError):
diff --git a/cylc/flow/platforms.py b/cylc/flow/platforms.py
index a1b6ee9f121..211a278072a 100644
--- a/cylc/flow/platforms.py
+++ b/cylc/flow/platforms.py
@@ -21,7 +21,7 @@
 from copy import deepcopy
 from typing import (
     TYPE_CHECKING, Any, Dict, Iterable,
-    List, Optional, Set, Tuple, Union, overload
+    List, Optional, Set, Union, overload
 )
 
 from cylc.flow import LOG
diff --git a/cylc/flow/task_remote_mgr.py b/cylc/flow/task_remote_mgr.py
index 9671446e286..6d5dd6ecf63 100644
--- a/cylc/flow/task_remote_mgr.py
+++ b/cylc/flow/task_remote_mgr.py
@@ -32,11 +32,12 @@
 from subprocess import Popen, PIPE, DEVNULL
 import tarfile
 from time import sleep, time
-from typing import Any, Deque, Dict, TYPE_CHECKING, List, NamedTuple, Tuple
+from typing import (
+    Any, Deque, Dict, TYPE_CHECKING, List, NamedTuple, Set, Tuple)
 
 from cylc.flow import LOG
 from cylc.flow.exceptions import (
-    PlatformError, PlatformLookupError, NoHostsError
+    PlatformError, PlatformLookupError, NoHostsError, NoPlatformsError
 )
 import cylc.flow.flags
 from cylc.flow.hostuserutil import is_remote_host
@@ -290,7 +291,7 @@ def construct_remote_tidy_ssh_cmd(
     @staticmethod
     def _get_remote_tidy_targets(
         platform_names: List[str],
-        install_targets: List[str]
+        install_targets: Set[str]
     ) -> Dict[str, List[Dict[str, Any]]]:
         """Finds valid platforms for install targets, warns about in invalid
         install targets.
@@ -316,10 +317,6 @@ def _get_remote_tidy_targets(
         if unreachable_targets:
             msg = 'No platforms available to remote tidy install targets:'
             for unreachable_target in unreachable_targets:
-                msg = (
-                    'Unable to tidy the following install targets'
-                    ' because no matching platform was found:'
-                )
                 msg += f'\n * {unreachable_target}'
             LOG.error(msg)
 
@@ -346,10 +343,10 @@ def remote_tidy(self) -> None:
 
         # Issue all SSH commands in parallel
         queue: Deque[RemoteTidyQueueTuple] = deque()
-        for install_target in install_targets_map.keys():
+        for install_target, platforms in install_targets_map.items():
             if install_target == get_localhost_install_target():
                 continue
-            for platform in install_targets_map[install_target]:
+            for platform in platforms:
                 try:
                     cmd, host = self.construct_remote_tidy_ssh_cmd(platform)
                 except NoHostsError as exc:
@@ -372,6 +369,11 @@ def remote_tidy(self) -> None:
                         )
                     )
                     break
+            else:
+                LOG.error(
+                    # f'Unable to clean install target {install_target}',
+                    NoPlatformsError(install_target, 'install target')
+                )
         # Wait for commands to complete for a max of 10 seconds
         timeout = time() + 10.0
         while queue and time() < timeout: