attempt to handle rose rsync can_pull errors better

handle no file error with useful error message

Apply suggestions from code review

Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>

Response to review

move error messaging into error class

Author: wxtim

changes.d/2892.fix.md

@@ -0,0 +1 @@
+Give the user more information about rsync file handler failure.

metomi/rose/config_processors/fileinstall.py

@@ -891,12 +891,15 @@ def parse(self, loc, conf_tree):
             if handler is None:
                 raise ValueError(f"don't support scheme {loc.scheme}")
         else:
-            # Scheme not specified in the configuration.
+            # Try to get the scheme by parsing loc name, e.g. git:some-url
             scheme = urlparse(loc.name).scheme
             if scheme:
                 handler = self.get_handler(scheme)
                 if handler is None:
+                    # Try to guess the scheme using the ``can_handle`` method
+                    # from each handler in turn:
                     handler = self.guess_handler(loc)
+
                 if handler is None:
                     raise ValueError(f"don't know how to process {loc.name}")
             else:

metomi/rose/loc_handlers/rsync.py

@@ -16,6 +16,8 @@
 # -----------------------------------------------------------------------------
 """A handler of locations on remote hosts."""
 
+from io import TextIOWrapper
+from textwrap import indent
 from time import sleep, time
 
 from metomi.rose.loc_handlers.rsync_remote_check import (
@@ -24,6 +26,58 @@
 from metomi.rose.popen import RosePopenError
 
 
+class PreRsyncCheckError(Exception):
+    """Error to raise if we assume the loc is for use with rsync,
+    but attempting to use it as an rsync loc fails."""
+    BASE_MESSAGE = (
+        'Rose tried all other file install handlers and'
+        ' decided this must be an Rsync handler.\n\t'
+    )
+
+    def __init__(self, dict_, cmd=None):
+        for key, value in dict_.items():
+            if isinstance(value, TextIOWrapper):
+                setattr(self, key, value.read())
+            else:
+                setattr(self, key, value)
+
+        # Convert command into something the debugger can try:
+        try:
+            self.cmd = ' '.join(cmd)
+        except TypeError:
+            self.cmd = cmd
+
+        # Handle ``test -e nonexistant`` where no useful error is
+        # provided:
+        if (
+            self.returncode == 1
+            and self.stderr == ''
+            and self.stdout == ''
+        ):
+            self.stderr = f'File "{cmd[-1]}" does not exist.'
+
+        if self.returncode == 255:
+            host = dict_['args'][dict_['args'].index('-n') + 1]
+            self.message = (
+                self.BASE_MESSAGE
+                + 'If it is then host'
+                f' "{host}"'
+                ' is uncontactable (ssh 255 error).'
+            )
+        else:
+            self.message = (
+                self.BASE_MESSAGE
+                + f'`{self.cmd}` failed with:'
+                + indent(
+                    f'\nreturncode: {self.returncode}'
+                    f'\nstdout:     {self.stdout}'
+                    f'\nstderr:     {self.stderr}',
+                    prefix='    ',
+                )
+            )
+        super().__init__(self.message)
+
+
 class RsyncLocHandler:
     """Handler of locations on remote hosts."""
 
@@ -53,7 +107,10 @@ def can_pull(self, loc):
         except RosePopenError:
             return False
         else:
-            return proc.wait() == 0
+            if proc.wait() == 0:
+                return True
+            else:
+                raise PreRsyncCheckError(proc.__dict__, cmd=cmd)
 
     def parse(self, loc, _):
         """Set loc.scheme, loc.loc_type, loc.paths."""

metomi/rose/scheme_handler.py

@@ -97,6 +97,11 @@ def __init__(
                             if handler is None:
                                 handler = class_(*args, **kwargs)
                             self.handlers[scheme] = handler
+
+                if 'rsync' in self.handlers:
+                    # rsync handler should always be at the end of the list:
+                    self.handlers['rsync'] = self.handlers.pop('rsync')
+
             finally:
                 os.chdir(cwd)
                 sys.path.pop(0)

t/rose-app-run/05-file.t

@@ -70,7 +70,8 @@ run_fail "$TEST_KEY" rose app-run --config=../config -q \
     --define='[file:hello4]source=stuff:ing'
 file_cmp "$TEST_KEY.out" "$TEST_KEY.out" </dev/null
 file_cmp "$TEST_KEY.err" "$TEST_KEY.err" <<'__CONTENT__'
-[FAIL] file:hello4=source=stuff:ing: don't know how to process stuff:ing
+[FAIL] Rose tried all other file install handlers and decided this must be an Rsync handler.
+[FAIL] 	If it is then host "stuff" is uncontactable (ssh 255 error).
 __CONTENT__
 test_teardown
 #-------------------------------------------------------------------------------