Merge remote-tracking branch 'upstream/8.1.x' into fix.platform_is_re…

…gex_remote_tidy_fail

* upstream/8.1.x:
  Update cylc/flow/scripts/message.py
  Upload coverage to Codecov in separate job (cylc#5459)
  upgrade cylc message internal help with details of severity levels
  Update tests/functional/platforms/10-do-not-host-check-platforms.t
  Fix flake8-comprehensions C419 Don't use any([i for i in iterable]) use any(i for i in iterable). It's more efficient because we don't have to expand the entire thing.
  Improve readability of host/platform eval code (#53)
  small changlog error fix
  update comment on localhost check and add test for case localhost4.localhost42
  undo mistake
  clarification of nomenclature
  Avoid running host check on platform names - this doesn't make any sense.

.github/workflows/test_fast.yml

@@ -35,20 +35,6 @@ jobs:
         with:
           python-version: ${{ matrix.python-version }}
 
-      - name: Brew Install
-        if: startsWith(matrix.os, 'macos')
-        run: |
-          # speed up install (https://docs.brew.sh/Manpage#environment)
-          export HOMEBREW_NO_AUTO_UPDATE=1 HOMEBREW_NO_INSTALL_CLEANUP=1
-          echo "[command]brew update"
-          brew update
-          echo "[command]brew install ..."
-          brew install bash coreutils
-          # add GNU coreutils and sed to the user PATH
-          # (see instructions in brew install output)
-          echo "$(brew --prefix)/opt/coreutils/libexec/gnubin" \
-            >> "${GITHUB_PATH}"
-
       - name: Apt-Get Install
         if: startsWith(matrix.os, 'ubuntu')
         run: |
@@ -93,7 +79,7 @@ jobs:
         run: |
           pytest tests/integration
 
-      - name: Upload artifact
+      - name: Upload failed tests artifact
         if: failure()
         uses: actions/upload-artifact@v3
         with:
@@ -105,15 +91,35 @@ jobs:
           coverage xml
           coverage report
 
+      - name: Upload coverage artifact
+        uses: actions/upload-artifact@v3
+        with:
+          name: coverage_${{ matrix.os }}_py-${{ matrix.python-version }}
+          path: coverage.xml
+          retention-days: 7
+
+      - name: Linkcheck
+        if: startsWith(matrix.python-version, '3.10')
+        run: pytest -m linkcheck --dist=load tests/unit
+
+  codecov:
+    needs: test
+    runs-on: ubuntu-latest
+    timeout-minutes: 2
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v3
+
+      - name: Download coverage artifacts
+        uses: actions/download-artifact@v3
+
       - name: Codecov upload
         uses: codecov/codecov-action@v3
         with:
-          name: '${{ github.workflow }} ${{ matrix.os }} py-${{ matrix.python-version }}'
+          name: ${{ github.workflow }}
           flags: fast-tests
           fail_ci_if_error: true
           verbose: true
-          token: ${{ secrets.CODECOV_TOKEN }} # Token not required for public repos, but might reduce chance of random 404 error?
-
-      - name: Linkcheck
-        if: startsWith(matrix.python-version, '3.10')
-        run: pytest -m linkcheck --dist=load tests/unit
+          # Token not required for public repos, but avoids upload failure due
+          # to rate-limiting (but not for PRs opened from forks)
+          token: ${{ secrets.CODECOV_TOKEN }}

.github/workflows/test_functional.yml

@@ -248,14 +248,13 @@ jobs:
               -exec echo '====== {} ======' \; -exec cat '{}' \;
 
       - name: Set artifact upload name
-        if: failure() && steps.test.outcome == 'failure'
         id: uploadname
         run: |
           # artifact name cannot contain '/' characters
           CID="$(sed 's|/|-|g' <<< "${{ matrix.name || matrix.chunk }}")"
           echo "uploadname=$CID" >> $GITHUB_OUTPUT
 
-      - name: Upload artifact
+      - name: Upload failed tests artifact
         if: failure() && steps.test.outcome == 'failure'
         uses: actions/upload-artifact@v3
         with:
@@ -294,11 +293,31 @@ jobs:
           coverage xml
           coverage report
 
+      - name: Upload coverage artifact
+        uses: actions/upload-artifact@v3
+        with:
+          name: coverage_${{ steps.uploadname.outputs.uploadname }}
+          path: coverage.xml
+          retention-days: 7
+
+  codecov:
+    needs: test
+    runs-on: ubuntu-latest
+    timeout-minutes: 2
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v3
+
+      - name: Download coverage artifacts
+        uses: actions/download-artifact@v3
+
       - name: Codecov upload
         uses: codecov/codecov-action@v3
         with:
-          name: '${{ github.workflow }} ${{ matrix.name }} ${{ matrix.chunk }}'
+          name: ${{ github.workflow }}
           flags: functional-tests
           fail_ci_if_error: true
           verbose: true
-          token: ${{ secrets.CODECOV_TOKEN }} # Token not required for public repos, but might reduce chance of random 404 error?
+          # Token not required for public repos, but avoids upload failure due
+          # to rate-limiting (but not for PRs opened from forks)
+          token: ${{ secrets.CODECOV_TOKEN }}

CHANGES.md

@@ -30,6 +30,9 @@ Fixes a possible scheduler traceback observed with remote task polling.
 absence of `job name length maximum` in PBS platform settings would cause
 Cylc to crash when preparing the job script.
 
+[#5343](https://github.com/cylc/cylc-flow/pull/5343) - Fix a bug causing
+platform names to be checked as if they were hosts.
+
 [#5359](https://github.com/cylc/cylc-flow/pull/5359) - Fix bug where viewing
 a workflow's log in the GUI or using `cylc cat-log` would prevent `cylc clean`
 from working.

cylc/flow/hostuserutil.py

@@ -197,8 +197,8 @@ def is_remote_host(self, name):
 
         """
         if name not in self._remote_hosts:
-            if not name or name.split(".")[0].startswith("localhost"):
-                # e.g. localhost.localdomain
+            if not name or name.startswith("localhost"):
+                # e.g. localhost4.localdomain4
                 self._remote_hosts[name] = False
             else:
                 try:

cylc/flow/option_parsers.py

@@ -104,7 +104,7 @@ def __sub__(self, other):
     def _in_list(self, others):
         """CLI arguments for this option found in any of a list of
         other options."""
-        return any([self & other for other in others])
+        return any(self & other for other in others)
 
     def _update_sources(self, other):
         """Update the sources from this and 1 other OptionSettings object"""

cylc/flow/platforms.py

@@ -488,10 +488,10 @@ def generic_items_match(
         # Get a set of items actually set in both platform and task_section.
         shared_items = set(task_section).intersection(set(platform_spec))
         # If any set items do not match, we can't use this platform.
-        if not all([
+        if not all(
             platform_spec[item] == task_section[item]
             for item in shared_items
-        ]):
+        ):
             return False
     return True
 

cylc/flow/scripts/lint.py

@@ -273,7 +273,7 @@ def get_pyproject_toml(dir_):
             raise CylcError(f'pyproject.toml did not load: {exc}')
 
         if any(
-            [i in loadeddata for i in ['cylc-lint', 'cylclint', 'cylc_lint']]
+            i in loadeddata for i in ['cylc-lint', 'cylclint', 'cylc_lint']
         ):
             for key in keys:
                 tomldata[key] = loadeddata.get('cylc-lint').get(key, [])

cylc/flow/scripts/message.py

@@ -59,6 +59,18 @@
 The default message severity is INFO. The --severity=SEVERITY option can be
 used to set the default severity level for all unprefixed messages.
 
+Increased severity will make messages more visible in workflow logs, using
+colour and format changes. DEBUG messages will not be shown in logs by default.
+
+The severity levels are those of the Python Logging Library
+https://docs.python.org/3/library/logging.html#logging-levels:
+
+- CRITICAL
+- ERROR
+- WARNING
+- INFO
+- DEBUG
+
 Note:
   To abort a job script with a custom error message, use cylc__job_abort:
     cylc__job_abort 'message...'

cylc/flow/task_job_mgr.py

@@ -58,8 +58,6 @@
 )
 from cylc.flow.pathutil import get_remote_workflow_run_job_dir
 from cylc.flow.platforms import (
-    HOST_REC_COMMAND,
-    PLATFORM_REC_COMMAND,
     get_host_from_platform,
     get_install_target_from_platform,
     get_localhost_install_target,
@@ -1120,12 +1118,12 @@ def _prep_submit_task_job(
         host_n, platform_name = None, None
         try:
             if rtconfig['remote']['host'] is not None:
-                host_n = self.task_remote_mgr.subshell_eval(
-                    rtconfig['remote']['host'], HOST_REC_COMMAND
+                host_n = self.task_remote_mgr.eval_host(
+                    rtconfig['remote']['host']
                 )
             else:
-                platform_name = self.task_remote_mgr.subshell_eval(
-                    rtconfig['platform'], PLATFORM_REC_COMMAND
+                platform_name = self.task_remote_mgr.eval_platform(
+                    rtconfig['platform']
                 )
         except PlatformError as exc:
             itask.waiting_on_job_prep = False

cylc/flow/task_remote_mgr.py

@@ -33,14 +33,15 @@
 import tarfile
 from time import sleep, time
 from typing import (
-    Any, Deque, Dict, TYPE_CHECKING, List, NamedTuple, Set, Tuple)
+    Any, Deque, Dict, TYPE_CHECKING, List,
+    NamedTuple, Optional, Tuple
+)
 
 from cylc.flow import LOG
 from cylc.flow.exceptions import (
     PlatformError, PlatformLookupError, NoHostsError, NoPlatformsError
 )
 import cylc.flow.flags
-from cylc.flow.hostuserutil import is_remote_host
 from cylc.flow.network.client_factory import CommsMeth
 from cylc.flow.pathutil import (
     get_dirs_to_symlink,
@@ -49,6 +50,10 @@
     get_workflow_run_dir,
 )
 from cylc.flow.platforms import (
+    HOST_REC_COMMAND,
+    PLATFORM_REC_COMMAND,
+    NoHostsError,
+    PlatformLookupError,
     get_host_from_platform,
     get_install_target_from_platform,
     get_install_target_to_platforms_map,
@@ -68,6 +73,7 @@
 )
 
 from cylc.flow.loggingutil import get_next_log_number, get_sorted_logs_by_time
+from cylc.flow.hostuserutil import is_remote_host
 
 if TYPE_CHECKING:
     from zmq.auth.thread import ThreadAuthenticator
@@ -108,74 +114,83 @@ def __init__(self, workflow, proc_pool, bad_hosts, db_mgr):
         self.is_restart = False
         self.db_mgr = db_mgr
 
-    def subshell_eval(self, command, command_pattern, host_check=True):
-        """Evaluate a task platform from a subshell string.
-
-        At Cylc 7, from a host string.
+    def _subshell_eval(
+        self, eval_str: str, command_pattern: re.Pattern
+    ) -> Optional[str]:
+        """Evaluate a platform or host from a possible subshell string.
 
         Arguments:
-            command (str):
-                An explicit host name, a command in back-tick or $(command)
-                format, or an environment variable holding a hostname.
-            command_pattern (re.Pattern):
+            eval_str:
+                An explicit host/platform name, a command, or an environment
+                variable holding a host/patform name.
+            command_pattern:
                 A compiled regex pattern designed to match subshell strings.
-            host_check (bool):
-                A flag to enable remote testing. If True, and if the command
-                is running locally, then it will return 'localhost'.
 
-        Return (str):
+        Return:
             - None if evaluation of command is still taking place.
-            - If command is not defined or the evaluated name is equivalent
-              to 'localhost', _and_ host_check is set to True then
-              'localhost'
-            - Otherwise, return the evaluated host name on success.
+            - 'localhost' if string is empty/not defined.
+            - Otherwise, return the evaluated host/platform name on success.
 
         Raise PlatformError on error.
 
         """
-        # BACK COMPAT: references to "host"
-        # remove at:
-        #     Cylc8.x
-        if not command:
+        if not eval_str:
             return 'localhost'
 
         # Host selection command: $(command) or `command`
-        match = command_pattern.match(command)
+        match = command_pattern.match(eval_str)
         if match:
             cmd_str = match.groups()[1]
             if cmd_str in self.remote_command_map:
                 # Command recently launched
                 value = self.remote_command_map[cmd_str]
                 if isinstance(value, PlatformError):
                     raise value  # command failed
-                elif value is None:
-                    return  # command not yet ready
-                else:
-                    command = value  # command succeeded
+                if value is None:
+                    return None  # command not yet ready
+                eval_str = value  # command succeeded
             else:
                 # Command not launched (or already reset)
                 self.proc_pool.put_command(
                     SubProcContext(
                         'remote-host-select',
                         ['bash', '-c', cmd_str],
-                        env=dict(os.environ)),
+                        env=dict(os.environ)
+                    ),
                     callback=self._subshell_eval_callback,
                     callback_args=[cmd_str]
                 )
                 self.remote_command_map[cmd_str] = None
-                return self.remote_command_map[cmd_str]
+                return None
 
         # Environment variable substitution
-        command = os.path.expandvars(command)
-        # Remote?
-        # TODO - Remove at Cylc 8.x as this only makes sense with host logic
-        if host_check is True:
-            if is_remote_host(command):
-                return command
-            else:
-                return 'localhost'
-        else:
-            return command
+        return os.path.expandvars(eval_str)
+
+    # BACK COMPAT: references to "host"
+        # remove at:
+        #     Cylc8.x
+    def eval_host(self, host_str: str) -> Optional[str]:
+        """Evaluate a host from a possible subshell string.
+
+        Args:
+            host_str: An explicit host name, a command in back-tick or
+                $(command) format, or an environment variable holding
+                a hostname.
+
+        Returns 'localhost' if evaluated name is equivalent
+        (e.g. localhost4.localdomain4).
+        """
+        host = self._subshell_eval(host_str, HOST_REC_COMMAND)
+        return host if is_remote_host(host) else 'localhost'
+
+    def eval_platform(self, platform_str: str) -> Optional[str]:
+        """Evaluate a platform from a possible subshell string.
+
+        Args:
+            platform_str: An explicit platform name, a command in $(command)
+                format, or an environment variable holding a platform name.
+        """
+        return self._subshell_eval(platform_str, PLATFORM_REC_COMMAND)
 
     def subshell_eval_reset(self):
         """Reset remote eval subshell results.