Review Feedback HO

Co-authored-by: Hilary James Oliver <[email protected]>

cylc/flow/cfgspec/globalcfg.py

@@ -372,13 +372,12 @@
         # Symlink Dirs
         with Conf('symlink dirs',  # noqa: SIM117 (keep same format)
                   desc="""
-            Define directories to be moved, symlinks from the the original
-            ``$HOME/cylc-run`` directories will be created.
+            Configure alternate workflow run directory locations. Symlinks from
+            the the standard ``$HOME/cylc-run`` locations will be created.
         """):
             with Conf('<install target>'):
                 Conf('run', VDR.V_STRING, None, desc="""
-                    Specifies the directory where the workflow run directories
-                    are created. If specified, the workflow run directory will
+                    If specified, the workflow run directory will
                     be created in ``<run dir>/cylc-run/<workflow-name>`` and a
                     symbolic link will be created from
                     ``$HOME/cylc-run/<workflow-name>``.
@@ -388,7 +387,6 @@
                     installed into this directory.
                 """)
                 Conf('log', VDR.V_STRING, None, desc="""
-                    Specifies the directory where log directories are created.
                     If specified the workflow log directory will be created in
                     ``<log dir>/cylc-run/<workflow-name>/log`` and a symbolic
                     link will be created from
@@ -397,17 +395,15 @@
                     ``$HOME/cylc-run/<workflow-name>/log``.
                 """)
                 Conf('share', VDR.V_STRING, None, desc="""
-                    Specifies the directory where share directories are
-                    created. If specified the workflow share directory will be
+                    If specified the workflow share directory will be
                     created in ``<share dir>/cylc-run/<workflow-name>/share``
                     and a symbolic link will be created from
                     ``<$HOME/cylc-run/<workflow-name>/share``. If not specified
                     the workflow share directory will be created in
                     ``$HOME/cylc-run/<workflow-name>/share``.
                 """)
                 Conf('share/cycle', VDR.V_STRING, None, desc="""
-                    Specifies the directory where share/cycle directories are
-                    created. If specified the workflow share/cycle directory
+                    If specified the workflow share/cycle directory
                     will be created in
                     ``<share/cycle dir>/cylc-run/<workflow-name>/share/cycle``
                     and a symbolic link will be created from
@@ -416,7 +412,6 @@
                     created in ``$HOME/cylc-run/<workflow-name>/share/cycle``.
                 """)
                 Conf('work', VDR.V_STRING, None, desc="""
-                    Specifies the directory where work directories are created.
                     If specified the workflow work directory will be created in
                     ``<work dir>/cylc-run/<workflow-name>/work`` and a symbolic
                     link will be created from

cylc/flow/workflow_files.py

@@ -37,6 +37,7 @@
     PlatformLookupError,
     ServiceFileError,
     TaskRemoteMgmtError,
+    UserInputError,
     WorkflowFilesError)
 from cylc.flow.pathutil import (
     expand_path,
@@ -562,7 +563,7 @@ def register(
     if not is_installed(get_workflow_run_dir(flow_name)):
         symlinks_created = make_localhost_symlinks(
             get_workflow_run_dir(flow_name), flow_name)
-        if bool(symlinks_created):
+        if symlinks_created:
             for src, dst in symlinks_created.items():
                 LOG.info(f"Symlink created from {src} to {dst}")
     # Create service dir if necessary.
@@ -586,11 +587,7 @@ def is_installed(rund: Union[Path, str]) -> bool:
     rund = Path(rund)
     cylc_install_dir = Path(rund, WorkflowFiles.Install.DIRNAME)
     alt_cylc_install_dir = Path(rund.parent, WorkflowFiles.Install.DIRNAME)
-    if cylc_install_dir.is_dir():
-        return True
-    elif alt_cylc_install_dir.is_dir():
-        return True
-    return False
+    return cylc_install_dir.is_dir() or alt_cylc_install_dir.is_dir()
 
 
 def _clean_check(reg, run_dir):
@@ -1162,7 +1159,7 @@ def install_workflow(
     symlinks_created = make_localhost_symlinks(
         rundir, named_run, symlink_conf=cli_symlink_dirs)
     install_log = _get_logger(rundir, 'cylc-install')
-    if bool(symlinks_created) is True:
+    if symlinks_created:
         for src, dst in symlinks_created.items():
             install_log.info(f"Symlink created from {src} to {dst}")
     try:
@@ -1374,15 +1371,22 @@ def get_sym_dirs(symlink_dirs: str) -> Dict[str, Dict[str, Any]]:
     symdict: Dict[str, Dict[str, Any]] = {'localhost': {'run': None}}
     if symlink_dirs == "":
         return symdict
-    symlist = symlink_dirs.replace(" ", "").strip(',').split(',')
+    symlist = symlink_dirs.strip(',').split(',')
     for pair in symlist:
         try:
             key, val = pair.split("=")
+            key = key.strip()
         except ValueError:
-            continue
+            raise UserInputError(
+                'There is an error in --symlink-dirs option:'
+                f' {pair}. Try entering option in the form '
+                '--symlink-dirs=\'log=$DIR, share=$DIR2, ...\''
+            )
         if key not in ['run', 'log', 'share', 'share/cycle', 'work']:
             raise WorkflowFilesError(
-                f"{key} not a valid entry for --symlink-dirs"
+                f"{key} not a valid entry for --symlink-dirs. "
+                "Configurable symlink dirs are: 'run', 'log', 'share',"
+                " 'share/cycle', 'work'"
             )
         symdict['localhost'][key] = val.strip() or None