Updating CI

.github/workflows/other.yml

@@ -45,40 +45,14 @@ jobs:
         env:
           SKIP: no-commit-to-branch
 
-      - name: Install dependencies only needed for python 2
-        if: ${{ matrix.python-version == '2.7' }}
-        run: pip install enum
-
-      - name: Install dependencies only needed for python 3
-        if: ${{ matrix.python-version != '2.7' }}
-        run: pip install opentelemetry-api opentelemetry-exporter-zipkin-json opentelemetry-sdk pandas pytype toml wrapt pyudev
-
-      - name: Install common dependencies for Python ${{matrix.python-version}}
-        run: pip install future mock pytest-coverage pytest-mock
-
       - name: Run Pytest for python 2 and get code coverage for Codecov
         if: ${{ matrix.python-version == '2.7' }}
         run: >
+          pip install enum future mock pytest-coverage pytest-mock &&
           pytest
-          --cov=scripts --cov=ocaml/xcp-rrdd
-          scripts/ ocaml/xcp-rrdd -vv -rA
-          --junitxml=.git/pytest${{matrix.python-version}}.xml
+          --cov=scripts scripts --cov-fail-under 45 -vv -rA
           --cov-report term-missing
           --cov-report xml:.git/coverage${{matrix.python-version}}.xml
-        env:
-          PYTHONDEVMODE: yes
-
-      - name: Run Pytest for python 3 and get code coverage for Codecov
-        if: ${{ matrix.python-version != '2.7' }}
-        run: >
-          pytest
-          --cov=scripts --cov=ocaml/xcp-rrdd --cov=python3/
-          scripts/ ocaml/xcp-rrdd python3/ -vv -rA
-          --junitxml=.git/pytest${{matrix.python-version}}.xml
-          --cov-report term-missing
-          --cov-report xml:.git/coverage${{matrix.python-version}}.xml
-        env:
-          PYTHONDEVMODE: yes
 
       - name: Upload Python ${{matrix.python-version}} coverage report to Codecov
         uses: codecov/codecov-action@v3
@@ -102,7 +76,7 @@ jobs:
 
       - name: Run pytype checks
         if: ${{ matrix.python-version != '2.7' }}
-        run: ./pytype_reporter.py
+        run: pip install pandas pytype toml && ./pytype_reporter.py
         env:
           PR_NUMBER: ${{ github.event.number }}
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

.pre-commit-config.yaml

@@ -14,6 +14,8 @@
 # pre-commit run -av --hook-stage pre-push
 #
 default_stages: [commit, push]
+default_language_version:
+    python: python3.11
 repos:
 # Recommendation for a minimal git pre-commit hook:
 # https://github.com/pre-commit/pre-commit-hooks/blob/main/README.md:
@@ -29,6 +31,95 @@ repos:
     -   id: check-executables-have-shebangs
         exclude: ocaml
 
+
+# Improve Python formatting incrementally:
+# https://dev.to/akaihola/improving-python-code-incrementally-3f7a
+#
+# darker checks if staged python changes are formatted according using
+# the PEP8-aligned black formatter. It also checks if the imports are sorted.
+#
+# It is a good idea to run this before committing, and it is also run in the
+# GitHub Workflow.
+#
+# Note: darker only checks the changes in files ending in .py!
+# Python scripts that don't end in .py should be renamed to have the .py extension
+# when moving them to python3/bin.
+# (remove the .py extension in the Makefile when installing the file)
+#
+-   repo: https://github.com/akaihola/darker
+    rev: 1.7.3
+    hooks:
+    -   id: darker
+        files: python3/
+        name: check changes in Python3 tree using darker and isort
+        args: [--diff, --skip-string-normalization, --isort, -tpy36]
+        additional_dependencies: [isort]
+
+#
+# Run pytest and diff-cover to check that the new /python3 test suite in passes.
+# This hook uses a local venv containing the required dependencies. When adding
+# new dependencies, they should be added to the additional_dependencies below.
+#
+-   repo: local
+    hooks:
+    -   id: pytest
+        files: python3/
+        name: check that the Python3 test suite in passes
+        entry: env PYTHONDEVMODE=yes sh -c 'python3 -m pytest -vv &&
+            diff-cover --ignore-whitespace --compare-branch=origin/feature/py3
+            --show-uncovered --html-report .git/coverage-diff.html
+            --fail-under 50 .git/coverage3.11.xml'
+        require_serial: true
+        pass_filenames: false
+        language: python
+        types: [python]
+        additional_dependencies:
+        - coverage
+        - diff-cover
+        - future
+        - opentelemetry-api
+        - opentelemetry-exporter-zipkin-json
+        - opentelemetry-sdk
+        - pytest-coverage
+        - pytest-mock
+        - mock
+        - wrapt
+        - XenAPI
+
+
+-   repo: https://github.com/RobertCraigie/pyright-python
+    rev: v1.1.361
+    hooks:
+    -   id: pyright
+        name: check that python3 tree passes pyright/VSCode check
+        files: python3/
+        additional_dependencies:
+        - mock
+        - opentelemetry-api
+        - opentelemetry-exporter-zipkin-json
+        - opentelemetry-sdk
+        - pytest
+        - pyudev
+        - XenAPI
+
+
+# Check that pylint passes for the changes in new /python3 code.
+-   repo: local
+    hooks:
+    -   id: pylint
+        files: python3/
+        stages: [push]
+        name: check that changes to python3 tree pass pylint
+        entry: diff-quality --violations=pylint
+            --ignore-whitespace --compare-branch=origin/feature/py3
+        pass_filenames: false
+        language: python
+        types: [python]
+        additional_dependencies: [diff-cover, pylint, pytest]
+
+
+# pre-push hook (it only runs if you install pre-commit as a pre-push hook):
+# It can be manually tested using: `pre-commit run -av --hook-stage push`
 # Recommendation for a minimal git pre-push hook:
 # While using pre-commit yields great results, it
 # is "not fast". Therefore only run it pre-push,
@@ -53,4 +144,12 @@ repos:
         # developers have such version installed, it can be configured here:
         # language_version: python3.11
         require_serial: true
-        additional_dependencies: [pandas, pytype]
+        additional_dependencies:
+        - future
+        - opentelemetry-api
+        - opentelemetry-exporter-zipkin-json
+        - opentelemetry-sdk
+        - pandas
+        - pytest
+        - pytype
+        files: python3/

pyproject.toml

@@ -34,11 +34,15 @@ line-length = 88
 
 [tool.coverage.report]
 # Here, developers can configure which lines do not need to be covered by tests:
+# fail_under:    minimum code coverage percentage
+fail_under = 50
+# exclude_lines: lines that are not required to be covered
 exclude_lines = [
     "pragma: no cover",  # standard pragma for not covering a line or block
     "if TYPE_CHECKING:", # imports for type checking only
     "pass",
     # Other specific lines that do not need to be covered, comment in which file:
+    "raise NbdDeviceNotFound",  # python3/libexec/usb_scan.py
 ]
 # precision digits to use when reporting coverage (sub-percent-digits are not reported):
 precision = 0
@@ -166,69 +170,73 @@ disable = [
 
 # -----------------------------------------------------------------------------
 # Pyright is the static analysis behind the VSCode Python extension / Pylance
-# https://microsoft.github.io/pyright/#/configuration?id=main-configuration-options
+# https://microsoft.github.io/pyright/#/configuration
 # -----------------------------------------------------------------------------
 
 [tool.pyright]
-# Specifies the paths of directories or files that should be included in the
-# analysis. If no paths are specified, all files in the workspace are included:
-include = ["python3", "ocaml/xcp-rrdd"]
-
-# Conditionalize the stube files for type definitions based on the platform:
-pythonPlatform = "Linux"
-
-# typeCheckingMode: "off", "basic", "standard" or "strict"
-typeCheckingMode = "standard"
-
-# Specifies the version of Python that will be used to execute the source code.
-# Generate errors if the source code makes use of language features that are
-# not supported in that version. It will also tailor its use of type stub files,
-# which conditionalizes type definitions based on the version. If no version is
-# specified, pyright will use the version of the current python interpreter,
-# if one is present:
-pythonVersion = "3.6"
-
-# Paths of directories or files that should use "strict" analysis if they are
-# included. This is the same as manually adding a "# pyright: strict" comment.
-# In strict mode, most type-checking rules are enabled, and the type-checker
-# will be more aggressive in inferring types. If no paths are specified, strict
-# mode is not enabled:
-strict = ["python3/tests/test_observer.py"]
-
-#
-# Paths to exclude from analysis. If a file is excluded, it will not be
-# analyzed.
-#
-# FIXME: Some of these may have type errors, so they should be inspected and fixed:
-#
-exclude = [
+# include:          directories to include in checking
+# strict:           paths for which strict checking works
+# typeCheckingMode: set the standard type checking mode
+include              = ["python3", "ocaml/xcp-rrdd"]
+strict               = ["python3/tests/observer"]
+pythonPlatform       = "Linux"
+typeCheckingMode     = "standard"
+reportMissingImports = false
+pythonVersion        = "3.6"
+exclude              = [
     "ocaml/xcp-rrdd/scripts/rrdd/rrdd.py",
     "ocaml/xcp-rrdd/scripts/rrdd/rrdd-example.py",
     "python3/packages/observer.py",
-    "python3/tests/pytype_reporter.py",
 ]
 
 
 # -----------------------------------------------------------------------------
 # Pytest is the test framework, for discovering and running tests, fixtures etc
-# https://pytest.readthedocs.io/en/latest/customize.html
+# https://pytest.readthedocs.io/en/latest/customize.html, https://docs.pytest.org
 # -----------------------------------------------------------------------------
 
-
 [tool.pytest.ini_options]
-addopts = "-ra" # Show the output of all tests, including those that passed
-log_cli = true  # Capture log messages and show them in the output as well
+# -----------------------------------------------------------------------------
+# Options to enable for pytest by default:
+# -v                           show what happens
+# -rA                          show summary after running tests
+# --cov=python3                measure coverage of the python3 directory
+# --cov-fail-under             minimum coverage percentage
+# --cov-report=term-missing    show missing lines in the coverage report
+# --cov-report=html:<filename> generate an HTML coverage report(for viewing)
+# --cov-report=xml:<filename>  generate an XML coverage report(for upload)
+# -----------------------------------------------------------------------------
+addopts = """
+-v -rA --cov=python3 --cov=scripts --cov-fail-under=50
+--cov-report=html:.git/coverage --cov-report=term-missing
+--cov-report=xml:.git/coverage3.11.xml
+"""
+
+# -----------------------------------------------------------------------------
+# Other pytest config options:
+# log_cli:          show logger messages
+# log_cli_level:    log level to show
+# python_files:     pattern for test files
+# python_functions: pattern for test functions
+# testpaths:        directories to search for tests
+# minversion:       this config requires pytest>=7 to configure pythonpath
+# pythonpath:       path to stub files and typing stubs for tests
+# xfail_strict:     require to remove pytext.xfail marker when test is fixed
+# required_plugins: require that these plugins are installed before testing
+# -----------------------------------------------------------------------------
+testpaths = ["python3", "scripts", "ocaml/xcp-rrdd"]
+required_plugins = ["pytest-cov", "pytest-mock"]
 log_cli_level = "INFO"
+log_cli = true
+minversion = "7.0"
+pythonpath = "python3/stubs:scripts/examples/python" # Allow to import the XenAPI module
 python_files = ["test_*.py", "it_*.py"]
 python_functions = ["test_", "it_", "when_"]
-pythonpath = "scripts/examples/python"  # Allows to import the XenAPI module
-required_plugins = ["pytest-mock"]
-testpaths = ["python3", "scripts", "ocaml/xcp-rrdd"]
 xfail_strict = true  # is used to fail tests that are marked as xfail but pass(for TDD)
 
 
 [tool.pytype_reporter]
-default_branch = "master"
+default_branch = "feature/py3"
 discard_messages_matching = [
     "Couldn't import pyi for 'xml.dom.minidom'",
     "No attribute '.*' on RRDContentHandler",
@@ -252,6 +260,10 @@ expected_to_fail = [
     "scripts/plugins/extauth-hook-AD.py",
 ]
 
+# -----------------------------------------------------------------------------
+# pytype: Google's static type analyzer - https://google.github.io/pytype/
+# -----------------------------------------------------------------------------
+
 [tool.pytype]
 inputs = [
     # Python 3

python3/bin/hfx_filename

@@ -14,8 +14,14 @@
 # ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
 # OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
+# pylint: disable=redefined-outer-name
+# pyright: reportFunctionMemberAccess=false
+# pyright: reportOptionalMemberAccess=false, reportAttributeAccessIssue=false
 
-import sys, socket, urllib.request, XenAPI
+import sys
+import socket
+
+import XenAPI
 
 db_url = "/remote_db_access"
 

python3/libexec/nbd_client_manager.py

@@ -136,7 +136,7 @@ def _find_unused_nbd_device():
             return nbd_device
 
     # If there are 1000 nbd devices (unlikely) and all are connected
-    raise NbdDeviceNotFound(nbd_device)
+    raise NbdDeviceNotFound(nbd_device)  # pyright:ignore[reportPossiblyUnboundVariable]
 
 def _wait_for_nbd_device(nbd_device, connected):
     deadline = datetime.now() + timedelta(minutes=MAX_DEVICE_WAIT_MINUTES)

python3/libexec/usb_reset.py

@@ -132,7 +132,7 @@ def load_device_ids(device):
         # ignore and continue
         log.warning("Failed to remove device ids: {}".format(str(e)))
 
-    return uid, gid
+    return uid, gid  # pyright: ignore[reportPossiblyUnboundVariable] # pragma: no cover
 
 
 # throw IOError, ValueError

python3/libexec/usb_scan.py

@@ -21,6 +21,8 @@
 # 2. check if device can be passed through based on policy file
 # 3. return the device info to XAPI in json format
 
+# pylint: disable=redefined-outer-name
+# pyright: reportPossiblyUnboundVariable=false, reportAttributeAccessIssue=false
 
 import abc
 import argparse
@@ -71,7 +73,7 @@ def __init__(self, node):
     def get_node(self):
         return self.node
 
-    def __hash__(self):
+    def __hash__(self):  # pyright:ignore[reportIncompatibleVariableOverride]
         return hash(self.node)
 
     def __eq__(self, other):
@@ -109,14 +111,14 @@ def _is_class_hub(self, key_class):
         return cls is not None and hex_equal(__VALUE_CLASS_HUB, cls)
 
     @abc.abstractmethod
-    def is_class_hub(self):
+    def is_class_hub(self) -> bool:
         """check if this belongs to a hub
 
         :return: bool, if this belongs to a hub
         """
 
     @abc.abstractmethod
-    def is_child_of(self, parent):
+    def is_child_of(self, parent) -> bool:
         """check if this is a child of parent
 
         :param parent:(UsbObject) the parent to check against
@@ -282,6 +284,7 @@ def __init__(self, node, props):
             if props.get(p) is not None:
                 self[p] = props.get(p).decode()
 
+    # pylint: disable-next=useless-parent-delegation # This parent call is superfluous
     def debug_str(self, level=0):
         return super().debug_str(level)
 

python3/packages/observer.py

@@ -41,7 +41,7 @@
 # We only want to import opentelemetry libraries if instrumentation is enabled
 # pylint: disable=import-outside-toplevel
 
-DEBUG_ENABLED = False
+DEBUG_ENABLED = os.getenv("XAPI_TEST")
 DEFAULT_MODULES = "LVHDSR,XenAPI,SR,SRCommand,util"
 FORMAT = "observer.py: %(message)s"
 handler = SysLogHandler(facility="local5", address="/dev/log")