To-be-split-up: Add pytype to CI to find Py3 bugs and fix issues to l…

…et pytype finish

Signed-off-by: Bernhard Kaindl <[email protected]>

.github/workflows/main.yml

@@ -73,6 +73,8 @@ jobs:
           python-version: '3.10'
           cache: 'pip'
 
+      - run: sudo apt-get install -y libcurl4-nss-dev
+
       - uses: actions/cache@v3
         name: Setup cache for running pre-commit fast
         with:

.pre-commit-config.yaml

@@ -57,5 +57,18 @@ repos:
     rev: v1.8.0
     hooks:
     -   id: mypy
-        files: ^scripts/unit_tests/
-        additional_dependencies: [pytest-mock, types-mock]
+        files: ^scripts/(unit_tests|usb_reset.py)
+        additional_dependencies: [pytest-mock, types-mock]
+
+
+-   repo: local
+    hooks:
+    -   id: pytype
+        name: pytype
+        entry: python3 run_pytype.py
+        types: [python]
+        verbose: true
+        language: python
+        pass_filenames: false
+        require_serial: true
+        additional_dependencies: [pandas, pytype, pyudev, XenAPI, urlgrabber]

.pylintrc

@@ -41,6 +41,7 @@ verbose=yes
 disable=
     bad-option-value, # old pylint for py2: ignore newer (unknown) pylint options
     bad-continuation, # old pylint warns about some modern black formatting
+    consider-using-f-string,
     useless-option-value, # new pylint has abaondoned these old options
 
 [BASIC]

Makefile

@@ -34,6 +34,10 @@ ifneq ($(PY_TEST), NO)
 	dune build @runtest-python --profile=$(PROFILE)
 endif
 
+pytype:
+	pip3 install --upgrade pytype
+	./run_pytype.py
+
 stresstest:
 	dune build @stresstest --profile=$(PROFILE) --no-buffer -j $(JOBS)
 

ocaml/message-switch/python/message_switch.py

@@ -273,7 +273,7 @@ def get_reply(self, correlation_id):
     def set_listen_callback(self, listen_callback):
         self.listen_callback = listen_callback
     def run(self):
-        ack_to = -1L
+        ack_to = -1
         timeout = 5.0
         while True:
             messages = transfer(self.sock, self.reader, ack_to, timeout)

pyproject.toml

@@ -0,0 +1,106 @@
+# https://packaging.python.org/en/latest/specifications/pyproject-toml/
+[project]
+name = "xen-api"
+requires-python = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*, !=3.5.*"
+license = {file = "LICENSE"}
+keywords = ["xen-project", "Xen", "hypervisor", "libraries"]
+maintainers = [
+    {name = "Christian Lindig"},
+    {name = "Edwin Török"},
+    {name = "Rob Hoes"},
+    {name = "Pau Ruiz Safont"},
+]
+readme = "README.markdown"
+# https://pypi.org/classifiers/
+classifiers = [
+    "Development Status :: 5 - Production/Stable",
+    "Operating System :: POSIX :: Linux :: XenServer Dom0",
+    "Operating System :: POSIX :: Linux :: XCP-ng Dom0",
+    "Programming Language :: ML",
+    "Programming Language :: Python :: Implementation :: CPython",
+]
+
+[project.urls]
+homepage = "https://github.com/xapi-project/xen-api"
+repository = "https://github.com/xapi-project/xen-api"
+
+[tool.black]
+line-length = 88
+
+[tool.isort]
+line_length = 88
+py_version = 27
+profile = "black"
+combine_as_imports = true
+ensure_newline_before_comments = false
+
+[tool.mypy]
+# Note mypy has no config setting for PYTHONPATH, so you need to call it with:
+# PYTHONPATH="scripts/examples/python:.:scripts:scripts/plugins:scripts/examples"
+files = [
+    "scripts/usb_reset.py",
+    "scripts/unit_tests",
+]
+pretty = true
+error_summary = true
+strict_equality = true
+show_error_codes = true
+show_error_context = true
+# Check the contents of untyped functions in all modules by default:
+check_untyped_defs = true
+scripts_are_modules = true
+python_version = "3.11"
+warn_return_any = true
+warn_unreachable = true
+warn_unused_configs = true
+warn_redundant_casts = true
+disallow_any_explicit = false
+disallow_any_generics = true
+disallow_any_unimported = true
+disallow_subclassing_any = true
+
+[tool.pytype]
+inputs = [
+    'scripts/*.py',
+    'scripts/',
+    "scripts/10resetvdis",
+    "scripts/Makefile",
+    "scripts/generate-iscsi-iqn",
+    "scripts/hatests",
+    "scripts/hfx_filename",
+    "scripts/host-display",
+    "scripts/mail-alarm",
+    "scripts/print-custom-templates",
+    "scripts/probe-device-for-file",
+    "scripts/xe-reset-networking",
+    "scripts/xe-scsi-dev-map",
+    'scripts/examples/python',
+    # Don't add, it can't do "from .XenAPI import *" afterwards:
+    # 'scripts/examples/python/XenAPI',
+    # Not yet ported:
+    # "ocaml/message-switch/python",
+    # "ocaml/idl/ocaml_backend/python",
+    # "ocaml/xapi-storage/python",
+]
+xfail = [
+    "scripts/perfmon",
+    "scripts/static-vdis",
+    "scripts/usb_scan.py",
+    "scripts/yum",
+    "scripts/yum/plugins.py",
+    "scripts/examples/python/monitor-unwanted-domains.py"
+]
+disable = [
+    'import-error',
+    "pyi-error",
+    "ignored-abstractmethod"
+]
+keepgoing = true
+platform = "linux"
+python_version = "3.10"
+pythonpath = "scripts/examples/python:.:scripts:scripts/plugins:scripts/examples"
+# pythonpath = "scripts/examples/python/XenAPI"
+# ":scripts/examples/python:scripts/examples:scripts:."
+# disable = ["ignored-type-comment"]
+# overriding_parameter_count_checks = true
+# use_enum_overlay = true

pytest.ini

@@ -0,0 +1,15 @@
+[pytest]
+
+# By default, show reports for failed tests:
+addopts=-rF
+
+# Enable log display during test run (also known as “live logging”).
+log_cli=True
+
+# Sets the minimum log message level that should be captured for live logging.
+# The integer value or the names of the levels can be used.
+# Lower it for debugging:
+log_cli_level=FATAL
+
+# When on path is passwd, run the tests below the scripts directory:
+testpaths=scripts/

run_pytype.py

@@ -0,0 +1,179 @@
+#!/usr/bin/env python
+import os
+import re
+import selectors
+import shlex
+import sys
+from logging import INFO, basicConfig, info
+from subprocess import PIPE, Popen
+from typing import Dict, List, TextIO, Tuple
+
+import pandas as pd  # type: ignore[import]
+from toml import load
+
+
+def generate_github_annotation(match: re.Match[str], branch_url: str) -> Tuple[str, Dict[str, str]]:
+    lineno = match.group(2)
+    code = match.group(5)
+    func = match.group(3)
+    msg = match.group(4)
+    assert isinstance(msg, str)
+    msg_splitpos = msg.find(" ", 21)
+    file = match.group(1)
+    linktext = os.path.basename(file).split(".")[0]
+    source_link = f"[`{linktext}`]({branch_url}/{file}#L{lineno})"
+    row = {
+        "Location": source_link,
+        "Function": f"`{func}`",
+        "Error code": code,
+        "Error message": msg[:msg_splitpos] + "<br>" + msg[msg_splitpos + 1 :],
+        "Error description": "",
+    }
+    # https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-an-error-message
+    return f"::error file={file},line={lineno},title=pytype: {code}::{msg}", row
+
+
+def filter_line(line, row):
+    if line.startswith("For more details, see"):
+        row["Error code"] = f"[{row['Error code']}]({line[22:]})"
+        return " " + line[22:]
+    if not row["Error description"]:
+        row["Error description"] = line.lstrip()
+    else:
+        row["Error description"] += " " + line.lstrip()
+    return ", " + line
+
+
+def skip_uninteresting_lines(line: str) -> bool:
+    if not line or line[0] == "/" or line.startswith("FAILED:"):
+        return True
+    if line[0] == "[":
+        pos = line.rfind(os.getcwd())
+        printfrom = pos + len(os.getcwd()) + 1 if pos > 0 else line.index("]") + 2
+        info("PROGRESS: " + line[1:].split("]")[0] + ": " + line[printfrom:])
+        return True
+    if line.startswith("ninja: "):
+        line = line[7:]
+    return bool(
+        (
+            line.startswith("Entering")
+            or line.startswith("Leaving")
+            or line.startswith("Computing")
+            or line.startswith("Analyzing")
+        )
+    )
+
+
+def run_pytype(command: List[str], branch_url: str, errorlog: TextIO, results):
+    info(" ".join(shlex.quote(arg) for arg in command))
+    # When run in tox, pytype dumps debug messages to stderr. Point stderr to /dev/null:
+    popen = Popen(command, stdout=PIPE, stderr=PIPE, universal_newlines=True)
+    assert popen.stdout and popen.stderr
+    error = ""
+    row = {}  # type: dict[str, str]
+    sel = selectors.DefaultSelector()
+    sel.register(popen.stdout, selectors.EVENT_READ)
+    sel.register(popen.stderr, selectors.EVENT_READ)
+    ok = True
+    while ok:
+        for key, _ in sel.select():
+            line = key.fileobj.readline()  # type: ignore
+            if not line:
+                ok = False
+                break
+            if key.fileobj is popen.stderr:
+                print(f"pytype: {line}", end="", file=sys.stderr)
+                continue
+            line = line.rstrip()
+            if skip_uninteresting_lines(line):
+                continue
+            info(line)
+            if row:
+                if line == "" or line[0] == " " or line.startswith("For more details, see"):
+                    if line:
+                        error += filter_line(line, row)
+                    continue
+                errorlog.write(
+                    error
+                    + " (you should find an entry in the pytype results with links below)\n"
+                )
+                results.append(row)
+                row = {}
+                error = ""
+            match = re.match(
+                r'File ".*libs/([^"]+)", line (\S+), in ([^:]+): (.*) \[(\S+)\]', line
+            )
+            if match:
+                error, row = generate_github_annotation(match, branch_url)
+    if popen.stdout:
+        popen.stdout.close()
+    popen.wait()
+    return popen.returncode, results
+
+
+def run_pytype_and_parse_annotations(xfail_files: List[str], branch_url: str):
+    """Send pytype errors to stdout and return results as pandas table
+
+    Args:
+        xfail_files (List[str]): list of files to exclude from pytype checks
+        branch_url (str): Base URL of the git branch for file links in github annotations
+    """
+    base_command = [
+        "pytype",
+        "-j",
+        "auto",
+    ]
+    if xfail_files:
+        exclude_command = ["--exclude", " ".join(xfail_files)]
+    else:
+        exclude_command = []
+
+    err_code, results = run_pytype(base_command + exclude_command, branch_url, sys.stderr, [])
+    if err_code or len(results):
+        return err_code if err_code > 0 else len(results), results
+    for xfail_file in xfail_files:
+        err_code, results = run_pytype(base_command + [xfail_file], branch_url, sys.stdout, results)
+        if err_code == 0:
+            print("No errors in", xfail_file)
+    return err_code or len(results), results
+
+def to_markdown(me, fp, returncode, results, branch_url):
+    mylink = f"[{me}]({branch_url}/{me}.py)"
+    pytype_link = "[pytype](https://google.github.io/pytype)"
+    if len(results) or returncode:
+        fp.write(f"\n#### {mylink} reports these {pytype_link} error messages:\n")
+        fp.write(pd.DataFrame(results).to_markdown())
+    else:
+        fp.write(f"\n#### Congratulations, {mylink} reports no {pytype_link} errors.\n")
+    fp.write("\n")
+
+
+def setup_and_run_pytype_action(script_name: str):
+    config = load("pyproject.toml")
+    pytype = config["tool"].get("pytype")
+    xfail_files = pytype.get("xfail", []) if pytype else []
+    repository_url = config["project"]["urls"]["repository"].strip(" /")
+    filelink_baseurl = repository_url + "/blob/master"
+
+    # When running as a GitHub action, we want to use URL of the fork with the GitHub action:
+    server_url = os.environ.get("GITHUB_SERVER_URL", None)
+    repository = os.environ.get("GITHUB_REPOSITORY", None)
+    if server_url and repository:
+        # https://github.com/orgs/community/discussions/5251 only set on Pull requests:
+        branch = os.environ.get("GITHUB_HEAD_REF", None) or os.environ.get("GITHUB_REF_NAME", None)
+        filelink_baseurl = f"{server_url}/{repository}/blob/{branch}"
+    ret_code, results = run_pytype_and_parse_annotations(xfail_files, filelink_baseurl)
+
+    # Write the panda table to a markdown output file:
+    summary_file = os.environ.get("GITHUB_STEP_SUMMARY", None)
+    if summary_file:
+        with open(summary_file, "w", encoding="utf-8") as fp:
+            to_markdown(script_name, fp, ret_code, results, filelink_baseurl)
+    else:
+        to_markdown(script_name, sys.stdout, ret_code, results, filelink_baseurl)
+
+
+if __name__ == "__main__":
+    script_basename = os.path.basename(__file__).split(".")[0]
+    basicConfig(format=script_basename + ": %(message)s", level=INFO)
+    sys.exit(setup_and_run_pytype_action(script_name = script_basename))