DAOS-14559 ci: use isort to organize python imports (#13243) (#13270)

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]>
Signed-off-by: Makito Kano <[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")

ci/functional/junit_list_unsuccessful

@@ -1,9 +1,10 @@
 #!/usr/bin/env python3
 """ list JUnit failures """
 
-from glob import glob
 import sys
-from junitparser import JUnitXml, Error, Failure
+from glob import glob
+
+from junitparser import Error, Failure, JUnitXml
 
 for file in glob(sys.argv[1]):
     for case in JUnitXml.fromfile(file):

ci/gha_helper.py

@@ -3,11 +3,11 @@
 """Helper module to choose build/cache keys to use for GitHub actions"""
 
 import os
-import sys
-from os.path import join
 import random
 import string
 import subprocess  # nosec
+import sys
+from os.path import join
 
 BUILD_FILES = ['site_scons/prereq_tools',
                'site_scons/components',

ci/jira_query.py

@@ -1,13 +1,14 @@
 #!/usr/bin/env python3
 """Interface between CI and bug-tracking tools"""
 
+import json
 import os
+import random
+import string
 import sys
-import json
 import time
 import urllib
-import random
-import string
+
 import jira
 
 # Script to improve interaction with Jenkins, GitHub and Jira.  This is intended to work in several

pyproject.toml

@@ -0,0 +1,5 @@
+[tool.isort]
+supported_extensions = ["py"]
+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/components/__init__.py

@@ -21,9 +21,10 @@
 """Defines common components used by HPDD projects"""
 
 import platform
+
 import distro
-from SCons.Script import GetOption
 from prereq_tools import GitRepoRetriever
+from SCons.Script import GetOption
 
 # Check if this is an ARM platform
 PROCESSOR = platform.machine()

site_scons/env_modules.py

@@ -19,12 +19,13 @@
 # SOFTWARE.
 """Wrapper for Modules so we can load an MPI before builds or tests"""
 
-import os
-import sys
 import errno
-import subprocess  # nosec
+import os
 import shutil
+import subprocess  # nosec
+import sys
 from subprocess import PIPE, Popen  # nosec
+
 import distro
 
 

site_scons/prereq_tools/base.py

@@ -20,28 +20,21 @@
 # -*- coding: utf-8 -*-
 """Classes for building external prerequisite components"""
 
-# pylint: disable=too-many-lines
-import os
-from copy import deepcopy
-import sys
-import json
+import configparser
 import datetime
-import traceback
 import errno
+import json
+# pylint: disable=too-many-lines
+import os
 import shutil
 import subprocess  # nosec
-import configparser
-from SCons.Variables import BoolVariable
-from SCons.Variables import EnumVariable
-from SCons.Variables import ListVariable
-from SCons.Variables import PathVariable
-from SCons.Script import Dir
-from SCons.Script import Exit
-from SCons.Script import GetOption
-from SCons.Script import SetOption
-from SCons.Script import WhereIs
-from SCons.Script import BUILD_TARGETS
+import sys
+import traceback
+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
 
 
 class DownloadFailure(Exception):

site_scons/site_tools/compiler_setup.py

@@ -1,8 +1,6 @@
 """Common DAOS library for setting up the compiler"""
 
-from SCons.Script import GetOption, Exit
-from SCons.Script import Configure
-
+from SCons.Script import Configure, Exit, GetOption
 
 DESIRED_FLAGS = ['-fstack-usage',
                  '-Wno-sign-compare',

site_scons/site_tools/daos_builder.py

@@ -1,13 +1,9 @@
 """Common DAOS build functions"""
 import os
 
-from SCons.Subst import Literal
-from SCons.Script import Dir
-from SCons.Script import GetOption
-from SCons.Script import WhereIs
-from SCons.Script import Depends
-from SCons.Script import Exit
 from env_modules import load_mpi
+from SCons.Script import Depends, Dir, Exit, GetOption, WhereIs
+from SCons.Subst import Literal
 
 libraries = {}
 missing = set()

site_scons/site_tools/doneapi.py

@@ -3,13 +3,13 @@
 Hack to support oneapi version of Intel compilers
 
 """
-import sys
 import os
+import sys
 
+import SCons.Errors
 import SCons.Tool.gcc
 import SCons.Util
 import SCons.Warnings
-import SCons.Errors
 
 
 # pylint: disable=too-few-public-methods

site_scons/site_tools/extra/__init__.py

@@ -7,4 +7,4 @@
 
 
 # pylint: disable=unused-import
-from .extra import generate, exists  # noqa: F401
+from .extra import exists, generate  # noqa: F401

site_scons/site_tools/extra/extra.py

@@ -9,9 +9,9 @@
 This is used by scons to reformat automatically generated header files to be readable, but also
 outside of scons by the clang-format commit hook to check the version.
 """
-import subprocess  # nosec
-import re
 import os
+import re
+import subprocess  # nosec
 import sys
 
 from SCons.Builder import Builder

site_scons/site_tools/go_builder.py

@@ -1,11 +1,11 @@
 """DAOS functions for building go"""
 
-import subprocess  # nosec B404
+import json
 import os
 import re
-import json
+import subprocess  # nosec B404
 
-from SCons.Script import Configure, GetOption, Scanner, Glob, Exit, File
+from SCons.Script import Configure, Exit, File, GetOption, Glob, Scanner
 
 GO_COMPILER = 'go'
 MIN_GO_VERSION = '1.18.0'

site_scons/site_tools/protoc/__init__.py

@@ -20,7 +20,9 @@
 # SOFTWARE.
 
 import os
+
 import SCons.Builder
+
 # pylint: disable=too-few-public-methods,missing-class-docstring
 
 

site_scons/site_tools/stack_analyzer.py

@@ -5,9 +5,10 @@
 
 Analyze stack usage output
 """
-import os
 import argparse
 import atexit
+import os
+
 from SCons.Script import Exit
 
 

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

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

src/client/pydaos/pydaos_core.py

@@ -11,10 +11,7 @@
 import enum
 
 # pylint: disable-next=relative-beyond-top-level
-from . import pydaos_shim
-from . import DAOS_MAGIC
-from . import PyDError
-from . import DaosClient
+from . import DAOS_MAGIC, DaosClient, PyDError, pydaos_shim
 
 # Import Object class as an enumeration
 ObjClassID = enum.Enum(