DAOS-14559 ci: Create Actions jobs for isort linting and version chec…

…king. (#13258)

Create a new generic linting job and add isort checking.
Pin versions of python checking tools.
Add job to master that checks versions are up-to-date.

Fix isort options for line-length and blacklisting.

Resolve isort and codespell issues.

Overall this increases coverage with new checks, gives us a common
linting job which should be easier to manage that multiple ones and
allows us to run PRs with pinned versions to prevent upstream changes
blocking development but still fails master checking when updates are
available.

Signed-off-by: Ashley Pittman <[email protected]>

.github/workflows/linting.yml

@@ -0,0 +1,42 @@
+name: Linting
+
+# Always run on Pull Requests as then these checks can be marked as required.
+on:
+  push:
+    branches:
+      - master
+      - 'feature/*'
+      - 'release/*'
+  pull_request:
+
+jobs:
+  # Run isort on the tree.
+  # This checks .py files only so misses SConstruct and SConscript files are not checked, rather
+  # for these files check them afterwards.  The output-filter will not be installed for this part
+  # so regressions will be detected but not annotated.
+  isort:
+    name: Python isort
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          ref: ${{ github.event.pull_request.head.sha }}
+      - uses: actions/setup-python@v3
+      - uses: isort/isort-action@master
+        with:
+          requirementsFiles: "requirements.txt utils/cq/requirements.txt"
+      - name: Run on SConstruct file.
+        run: isort --check-only SConstruct
+      - name: Run on build files.
+        run: find . -name SConscript | xargs isort --check-only
+
+  log-check:
+    name: Logging macro checking
+    runs-on: ubuntu-22.04
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v4
+        with:
+          ref: ${{ github.event.pull_request.head.sha }}
+      - name: Check DAOS logging macro use.
+        run: ./utils/cq/d_logging_check.py --github src

.github/workflows/spelling.yml

@@ -10,11 +10,11 @@ jobs:
     steps:
       - name: Checkout code
         uses: actions/checkout@v3
+      - name: Install extra python packages
+        run: pip install --requirement utils/cq/requirements.txt
       - name: Run check
         uses: codespell-project/actions-codespell@master
         with:
           skip: ./src/control/vendor,./src/control/go.sum,./.git
           ignore_words_file: ci/codespell.ignores
           builtin: clear,rare,informal,names,en-GB_to_en-US
-      - name: Check DAOS logging macro use.
-        run: ./utils/cq/d_logging_check.py --github src

.github/workflows/version-checks.yml

@@ -0,0 +1,26 @@
+name: Version checking
+
+on:
+  push:
+    branches:
+      - master
+
+jobs:
+  upgrade-check:
+    name: Check for updates
+    runs-on: ubuntu-22.04
+    strategy:
+      fail-fast: false
+      matrix:
+        package: [pylint, yamllint, isort, codespell]
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v3
+      - name: Install extra python packages
+        run: python3 -m pip install --requirement utils/cq/requirements.txt
+      - name: Check ${{ matrix.package }} version
+        run: python -m ${{ matrix.package }} --version | tee -a version-pre
+      - name: Upgrade
+        run: pip install --upgrade ${{ matrix.package }}
+      - name: Check ${{ matrix.package }} for version
+        run: python -m  ${{ matrix.package }} --version | diff version-pre -

.github/workflows/yaml.yml

@@ -11,6 +11,7 @@ on:
     paths:
       - '**/*.yaml'
       - '**/*.yml'
+      - utils/cq/requirements.txt
 
 jobs:
   yaml-lint:
@@ -25,5 +26,7 @@ jobs:
         uses: actions/setup-python@v4
         with:
           python-version: '3'
+      - name: Install extra python packages
+        run: pip install --requirement utils/cq/requirements.txt
       - name: Run check
         run: yamllint --format github .

SConstruct

@@ -1,12 +1,12 @@
 """Build DAOS"""
+import errno
 import os
-import sys
 import subprocess  # nosec
+import sys
 import time
-import errno
+
 import SCons.Warnings
-from prereq_tools import PreReqComponent
-# pylint: disable=reimported
+from prereq_tools import PreReqComponent  # pylint: disable=reimported
 
 if sys.version_info.major < 3:
     print(""""Python 2.7 is no longer supported in the DAOS build.
@@ -177,8 +177,8 @@ def check_for_release_target():  # pylint: disable=too-many-locals
         # pylint: disable=consider-using-f-string
         try:
             # pylint: disable=import-outside-toplevel
-            import pygit2
             import github
+            import pygit2
             import yaml
         except ImportError:
             print("You need yaml, pygit2 and pygithub python modules to create releases")

pyproject.toml

@@ -1,3 +1,5 @@
 [tool.isort]
 supported_extensions = ["py"]
-skip = [".git/", "src/rdb/raft", "build", "install"]
+skip = [".git/", "src/rdb/raft", "build", "install", "venv", "src/control/vendor/"]
+line_length = 99
+skip_gitignore = true

requirements.txt

@@ -1,4 +1,4 @@
-# Requirements which are not versioned.
+# Packages required to build/test DAOS.
 defusedxml
 distro
 jira
@@ -8,6 +8,6 @@ ninja
 pyelftools
 pyxattr
 pyyaml
-scons        # Works around a bug in scons on EL 8.
+scons
 tabulate
 wheel

site_scons/prereq_tools/base.py

@@ -33,10 +33,8 @@
 from copy import deepcopy
 
 from SCons.Errors import InternalError
-from SCons.Script import (BUILD_TARGETS, Dir, Exit, GetOption, SetOption,
-                          WhereIs)
-from SCons.Variables import (BoolVariable, EnumVariable, ListVariable,
-                             PathVariable)
+from SCons.Script import BUILD_TARGETS, Dir, Exit, GetOption, SetOption, WhereIs
+from SCons.Variables import BoolVariable, EnumVariable, ListVariable, PathVariable
 
 
 class DownloadFailure(Exception):

src/cart/SConscript

@@ -4,6 +4,7 @@
 #
 """Build CaRT components"""
 from datetime import date
+
 import SCons.Action
 
 SRC = ['crt_bulk.c', 'crt_context.c', 'crt_corpc.c',

src/client/pydaos/__init__.py

@@ -9,8 +9,7 @@
 
 import atexit
 
-from . import \
-    pydaos_shim  # pylint: disable=relative-beyond-top-level,import-self
+from . import pydaos_shim  # pylint: disable=relative-beyond-top-level,import-self
 
 DAOS_MAGIC = 0x7A8A
 

src/control/SConscript

@@ -1,9 +1,9 @@
 """Build DAOS Control Plane"""
 # pylint: disable=too-many-locals
 import os
-from os.path import join
-from os import urandom
 from binascii import b2a_hex
+from os import urandom
+from os.path import join
 
 
 def is_firmware_mgmt_build(benv):

src/tests/ftest/config_file_gen.py

@@ -12,13 +12,11 @@
 import sys
 from argparse import ArgumentParser, RawDescriptionHelpFormatter
 
-from util.agent_utils_params import (DaosAgentTransportCredentials,
-                                     DaosAgentYamlParameters)
+from util.agent_utils_params import DaosAgentTransportCredentials, DaosAgentYamlParameters
 from util.command_utils_base import CommonConfig
 from util.dmg_utils_params import DmgTransportCredentials, DmgYamlParameters
 from util.exception_utils import CommandFailure
-from util.server_utils_params import (DaosServerTransportCredentials,
-                                      DaosServerYamlParameters)
+from util.server_utils_params import DaosServerTransportCredentials, DaosServerYamlParameters
 
 
 def generate_agent_config(args):

src/tests/ftest/control/dmg_network_scan.py

@@ -4,8 +4,7 @@
   SPDX-License-Identifier: BSD-2-Clause-Patent
 """
 from apricot import TestWithServers
-from network_utils import (SUPPORTED_PROVIDERS, get_dmg_network_information,
-                           get_network_information)
+from network_utils import SUPPORTED_PROVIDERS, get_dmg_network_information, get_network_information
 
 
 class DmgNetworkScanTest(TestWithServers):

src/tests/ftest/control/dmg_storage_query.py

@@ -8,8 +8,7 @@
 
 import avocado
 from control_test_base import ControlTestBase
-from dmg_utils import (get_storage_query_device_info,
-                       get_storage_query_pool_info)
+from dmg_utils import get_storage_query_device_info, get_storage_query_pool_info
 from exception_utils import CommandFailure
 from general_utils import dict_to_str, list_to_str
 

src/tests/ftest/deployment/critical_integration.py

@@ -10,8 +10,7 @@
 from apricot import TestWithoutServers, TestWithServers
 from ClusterShell.NodeSet import NodeSet
 from exception_utils import CommandFailure
-from general_utils import (DaosTestError, get_journalctl, journalctl_time,
-                           run_command)
+from general_utils import DaosTestError, get_journalctl, journalctl_time, run_command
 from ior_test_base import IorTestBase
 
 # pylint: disable-next=fixme

src/tests/ftest/harness/unit.py

@@ -5,8 +5,7 @@
 """
 from apricot import TestWithoutServers
 from ClusterShell.NodeSet import NodeSet
-from data_utils import (dict_extract_values, dict_subtract, list_flatten,
-                        list_stats, list_unique)
+from data_utils import dict_extract_values, dict_subtract, list_flatten, list_stats, list_unique
 from run_utils import ResultData, run_remote
 
 

src/tests/ftest/launch.py

@@ -31,28 +31,21 @@
 from slurm_setup import SlurmSetup, SlurmSetupException
 
 # Update the path to support utils files that import other utils files
-# This is not good coding practice. Should use package paths and remove alls these E402.
+# This is not good coding practice. Should use package paths and remove all these E402.
 sys.path.append(os.path.join(os.path.dirname(os.path.abspath(__file__)), "util"))
-from data_utils import (dict_extract_values, list_flatten,  # noqa: E402
-                        list_unique)
+from data_utils import dict_extract_values, list_flatten, list_unique  # noqa: E402
 # pylint: disable=import-outside-toplevel
-from host_utils import (HostException, HostInfo, get_local_host,  # noqa: E402
-                        get_node_set)
+from host_utils import HostException, HostInfo, get_local_host, get_node_set  # noqa: E402
 from logger_utils import get_console_handler, get_file_handler  # noqa: E402
-from results_utils import (Job, Results, TestResult, create_html,  # noqa: E402
-                           create_xml)
+from results_utils import Job, Results, TestResult, create_html, create_xml  # noqa: E402
 from run_utils import stop_processes  # noqa: E402
-from run_utils import (RunException, find_command, run_local,  # noqa: E402
-                       run_remote)
-from slurm_utils import (create_partition, delete_partition,  # noqa: E402
-                         show_partition)
+from run_utils import RunException, find_command, run_local, run_remote  # noqa: E402
+from slurm_utils import create_partition, delete_partition, show_partition  # noqa: E402
 from storage_utils import StorageException, StorageInfo  # noqa: E402
 from user_utils import get_user_groups  # noqa: E402
-from user_utils import (get_chown_command, get_group_id,  # noqa: E402
-                        groupadd, useradd, userdel)
+from user_utils import get_chown_command, get_group_id, groupadd, useradd, userdel  # noqa: E402
 from yaml_utils import YamlException  # noqa: E402
-from yaml_utils import (YamlUpdater, get_test_category,  # noqa: E402
-                        get_yaml_data)
+from yaml_utils import YamlUpdater, get_test_category, get_yaml_data  # noqa: E402
 
 BULLSEYE_SRC = os.path.join(os.path.dirname(os.path.abspath(__file__)), "test.cov")
 BULLSEYE_FILE = os.path.join(os.sep, "tmp", "test.cov")

src/tests/ftest/server/multiengine_persocket.py

@@ -6,8 +6,7 @@
 import base64
 import traceback
 
-from general_utils import (check_ping, check_ssh, get_random_bytes,
-                           wait_for_result)
+from general_utils import check_ping, check_ssh, get_random_bytes, wait_for_result
 from ior_test_base import IorTestBase
 from mdtest_test_base import MdtestBase
 from pydaos.raw import DaosApiError

src/tests/ftest/server/storage_tiers.py

@@ -8,8 +8,7 @@
 import yaml
 from apricot import TestWithServers
 from command_utils_base import CommonConfig
-from server_utils import (DaosServerTransportCredentials,
-                          DaosServerYamlParameters)
+from server_utils import DaosServerTransportCredentials, DaosServerYamlParameters
 
 
 class StorageTiers(TestWithServers):

src/tests/ftest/slurm_setup.py

@@ -22,8 +22,7 @@
 # pylint: disable=import-outside-toplevel
 from logger_utils import get_console_handler  # noqa: E402
 from package_utils import install_packages, remove_packages  # noqa: E402
-from run_utils import (command_as_user, get_clush_command,  # noqa: E402
-                       run_remote)
+from run_utils import command_as_user, get_clush_command, run_remote  # noqa: E402
 
 # Set up a logger for the console messages
 logger = logging.getLogger(__name__)

src/tests/ftest/util/agent_utils.py

@@ -8,12 +8,11 @@
 import socket
 from getpass import getuser
 
-from agent_utils_params import (DaosAgentTransportCredentials,
-                                DaosAgentYamlParameters)
+from agent_utils_params import DaosAgentTransportCredentials, DaosAgentYamlParameters
 from ClusterShell.NodeSet import NodeSet
 from command_utils import CommandWithSubCommand, SubprocessManager, YamlCommand
-from command_utils_base import (CommandWithParameters, CommonConfig,
-                                EnvironmentVariables, FormattedParameter)
+from command_utils_base import (CommandWithParameters, CommonConfig, EnvironmentVariables,
+                                FormattedParameter)
 from exception_utils import CommandFailure
 from general_utils import get_default_config_file, get_log_file, run_pcmd
 from run_utils import run_remote

src/tests/ftest/util/agent_utils_params.py

@@ -5,8 +5,7 @@
 """
 import os
 
-from command_utils_base import (BasicParameter, LogParameter,
-                                TransportCredentials, YamlParameters)
+from command_utils_base import BasicParameter, LogParameter, TransportCredentials, YamlParameters
 
 
 class DaosAgentTransportCredentials(TransportCredentials):

src/tests/ftest/util/apricot/apricot/__init__.py

@@ -1,5 +1,4 @@
 """apricot __init__."""
 __all__ = ['Test', 'TestWithServers', 'TestWithoutServers', 'skipForTicket']
 
-from apricot.test import (Test, TestWithoutServers, TestWithServers,
-                          skipForTicket)
+from apricot.test import Test, TestWithoutServers, TestWithServers, skipForTicket

src/tests/ftest/util/apricot/apricot/test.py

@@ -26,11 +26,9 @@
 from exception_utils import CommandFailure
 from fault_config_utils import FaultInjection
 from general_utils import (DaosTestError, dict_to_str, dump_engines_stacks,
-                           get_avocado_config_value, get_default_config_file,
-                           get_file_listing, nodeset_append_suffix, pcmd,
-                           run_command, set_avocado_config_value)
-from host_utils import (HostException, HostInfo, HostRole, get_host_parameters,
-                        get_local_host)
+                           get_avocado_config_value, get_default_config_file, get_file_listing,
+                           nodeset_append_suffix, pcmd, run_command, set_avocado_config_value)
+from host_utils import HostException, HostInfo, HostRole, get_host_parameters, get_local_host
 from job_manager_utils import get_job_manager
 from logger_utils import TestLogger
 from pydaos.raw import DaosApiError, DaosContext, DaosLog

src/tests/ftest/util/command_utils.py

@@ -16,13 +16,11 @@
 
 from avocado.utils import process
 from ClusterShell.NodeSet import NodeSet
-from command_utils_base import (BasicParameter, CommandWithParameters,
-                                EnvironmentVariables, LogParameter,
-                                ObjectWithParameters)
+from command_utils_base import (BasicParameter, CommandWithParameters, EnvironmentVariables,
+                                LogParameter, ObjectWithParameters)
 from exception_utils import CommandFailure
-from general_utils import (DaosTestError, change_file_owner, check_file_exists,
-                           create_directory, distribute_files,
-                           get_file_listing, get_job_manager_class,
+from general_utils import (DaosTestError, change_file_owner, check_file_exists, create_directory,
+                           distribute_files, get_file_listing, get_job_manager_class,
                            get_subprocess_stdout, run_command, run_pcmd)
 from run_utils import command_as_user
 from user_utils import get_primary_group

src/tests/ftest/util/daos_utils_base.py

@@ -4,8 +4,7 @@
   SPDX-License-Identifier: BSD-2-Clause-Patent
 """
 from command_utils import CommandWithSubCommand
-from command_utils_base import (BasicParameter, CommandWithParameters,
-                                FormattedParameter)
+from command_utils_base import BasicParameter, CommandWithParameters, FormattedParameter
 
 
 class DaosCommandBase(CommandWithSubCommand):