Fixup perfmon.11

.github/workflows/main.yml

@@ -194,3 +194,84 @@ jobs:
           github_token: ${{ secrets.github_token }}
           reporter: github-pr-review
           level: error
+
+      # uses: tsuyoshicho/action-mypy@v3.13.0
+      - uses: bernhardkaindl/action-mypy@36cb3a857d01c1bdaa2811893106c71580132d71
+        env:
+          MYPYPATH: scripts/examples/python:scripts/examples:scripts/plugins:scripts:.
+        with:
+          filter_mode: nofilter
+          setup_method: install
+          setup_command: pip install mypy mock
+          mypy_flags: |
+            --scripts-are-modules
+            --check-untyped-defs
+            --allow-redefinition
+            --hide-error-context
+            --ignore-missing-imports
+            --no-pretty
+            --warn-unreachable
+            --disable-error-code attr-defined
+            --disable-error-code call-overload
+            --disable-error-code import-not-found
+            --disable-error-code import-untyped
+            --disable-error-code type-arg
+            --disable-error-code var-annotated
+            --exclude ocaml/events/event_listen.py
+            --exclude ocaml/idl
+            --exclude ocaml/message-switch
+            --exclude ocaml/tests/tests/looper.py
+            --exclude ocaml/xcp-rrdd/scripts/rrdd/rrdd.py
+            --exclude scripts/examples
+            --exclude scripts/examples/python/monitor-unwanted-domains.py
+            --exclude scripts/scalability-tests/ping-master.py
+            --exclude scripts/backup-sr-metadata.py
+            --exclude scripts/restore-sr-metadata.py
+            --exclude scripts/nbd_client_manager.py
+          target: |
+            scripts/perfmon
+            scripts/hfx_filename
+            scripts/host-display
+          github_token: ${{ secrets.github_token }}
+          reporter: github-pr-review
+
+      - uses: dciborow/action-pylint@0.1.1
+        with:
+          filter_mode: nofilter
+          glob_pattern: scripts/examples/python/XenAPIPlugin.py
+          pylint_args: |
+            --verbose
+            --dsiable attribute-defined-outside-init
+            --disable bad-indentation
+            --disable consider-using-dict-comprehension
+            --disable consider-using-f-string
+            --disable consider-using-in
+            --disable consider-using-with
+            --disable global-statement
+            --disable import-error
+            --disable import-outside-toplevel
+            --disable invalid-name
+            --disable missing-final-newline
+            --disable missing-class-docstring
+            --disable missing-function-docstring
+            --dsiable missing-module-docstring
+            --disable multiple-imports
+            --disable multiple-statements
+            --disable no-else-return
+            --disable protected-access
+            --disable raise-missing-from
+            --disable redefined-outer-name
+            --disable too-few-public-methods
+            --disable too-many-branches
+            --disable too-many-statements
+            --disable trailing-whitespace
+            --disable try-except-raise
+            --disable use-dict-literal
+            --disable wrong-import-order
+            --max-line-length 112
+            scripts/perfmon
+            scripts/hfx_filename
+            scripts/test_perfmon.py
+            scripts/test_hfx_filename.py
+          github_token: ${{ secrets.github_token }}
+          reporter: github-pr-review

.pylintrc

@@ -0,0 +1,75 @@
+[MAIN]
+
+# Use multiple processes to speed up Pylint. Specifying 0 will auto-detect the
+# number of processors available to use, and will cap the count on Windows to
+# avoid hangs.
+jobs=2
+
+# Pickle collected data for later comparisons.
+persistent=yes
+
+# Discover python modules and packages in the file system subtree.
+recursive=yes
+
+# Add paths to the list of the source roots. Supports globbing patterns. The
+# source root is an absolute path or a path relative to the current working
+# directory used to determine a package namespace for modules located under the
+# source root.
+source-roots=
+    ocaml/xapi-storage/python,
+    scripts,
+    scripts/examples/python,
+
+# When enabled, pylint would attempt to guess common misconfiguration and emit
+# user-friendly hints instead of false-positive error messages.
+suggestion-mode=yes
+
+# In verbose mode, extra non-checker-related info will be displayed.
+verbose=yes
+
+[MESSAGES CONTROL]
+
+# Disable the message, report, category or checker with the given id(s). You
+# can either give multiple identifiers separated by comma (,) or put this
+# option multiple times (only on the command line, not in the configuration
+# file where it should appear only once). You can also use "--disable=all" to
+# disable everything first and then re-enable specific checks. For example, if
+# you want to run only the similarities checker, you can use "--disable=all
+# --enable=similarities". If you want to run only the classes checker, but have
+# no Warning level messages displayed, use "--disable=all --enable=classes
+# --disable=W".
+disable=
+    bad-option-value, # old pylint for py2: ignore newer (unknown) pylint options
+    bad-continuation, # old pylint warns about some modern black formatting
+    bad-indentation,
+    consider-using-dict-comprehension,
+    consider-using-f-string,
+    consider-using-with,
+    import-error,
+    import-outside-toplevel,
+    invalid-name,
+    missing-final-newline,
+    missing-function-docstring,
+    multiple-imports,
+    redefined-outer-name,
+    trailing-whitespace,  # enable this soon, after citical PRs are merged
+    useless-option-value, # new pylint has abaondoned these old options
+
+[FORMAT]
+
+# Maximum number of characters on a single line.
+max-line-length=98
+
+[BASIC]
+
+# Good variable names which should always be accepted, separated by a comma.
+good-names=a,
+           b,
+           c,
+           f,
+           i,
+           j,
+           k,
+           ex,
+           Run,
+           _

.sourcery.yaml

@@ -0,0 +1,5 @@
+rule_settings:
+  disable:
+  - replace-interpolation-with-fstring
+  - use-fstring-for-concatenation
+  - use-fstring-for-formatting

ocaml/xenopsd/scripts/swtpm-wrapper

@@ -87,7 +87,7 @@ def check_state_needs_init(fname):
         return False
 
     # Check if block device has non-zero header
-    with open(fname, "r") as file:
+    with open(fname, "rb") as file:
         hdr = file.read(8)
     int_hdr = struct.unpack("<Q", hdr)[0]
 

pyproject.toml

@@ -38,9 +38,21 @@ 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"
+exclude = [
+    "ocaml/xapi-storage/python/examples/volume/org.xen.xapi.storage.simple-file/plugin.py",
+    "scripts/examples",
+    "scripts/examples/python/monitor-unwanted-domains.py",
+    "scripts/scalability-tests/ping-master.py",
+    "scripts/backup-sr-metadata.py",
+    "scripts/restore-sr-metadata.py",
+    "scripts/nbd_client_manager.py",
+]
 files = [
+    "scripts/perfmon",
+    "scripts/hfx_filename",
+    "scripts/mail-alarm",
+    "scripts/host-display",
     "scripts/usb_reset.py",
-    "scripts/unit_tests",
 ]
 pretty = true
 error_summary = true
@@ -70,8 +82,6 @@ discard_messages_matching = [
     "No Node.TEXT_NODE in module xml.dom.minidom, referenced from 'xml.dom.expatbuilder'"
 ]
 expected_to_fail = [
-    "scripts/hfx_filename",
-    "scripts/perfmon",
     # Need 2to3 -w <file> and maybe a few other minor updates:
     "scripts/hatests",
     "scripts/backup-sr-metadata.py",
@@ -112,13 +122,12 @@ inputs = [
     "scripts/xe-scsi-dev-map",
     "scripts/examples/python",
     "scripts/yum-plugins",
-    "scripts/*.py",
-
     # To be added later,
     # when converted to Python3-compatible syntax:
     # "ocaml/message-switch/python",
     # "ocaml/idl/ocaml_backend/python",
     # "ocaml/xapi-storage/python",
+    "ocaml/xenopsd/python",
 ]
 disable = [
     "import-error",  # xenfsimage, xcp.bootloader. xcp.cmd

scripts/hfx_filename

@@ -28,18 +28,14 @@ def rpc(session_id, request):
         headers = [
           "POST %s?session_id=%s HTTP/1.0" % (db_url, session_id),
           "Connection:close",
-          "content-length:%d" % (len(request)),
+          "content-length:%d" % (len(request.encode('utf-8'))),
           ""
         ]
-        #print "Sending HTTP request:"
         for h in headers:
-          s.send("%s\r\n" % h)
-          #print "%s\r\n" % h,
-        s.send(request)
+          s.send((h + "\r\n").encode('utf-8'))
+        s.send(request.encode('utf-8'))
 
-        result = s.recv(1024)
-        #print "Received HTTP response:"
-        #print result
+        result = s.recv(1024).decode('utf-8')
         if "200 OK" not in result:
           print("Expected an HTTP 200, got %s" % result, file=sys.stderr)
           return
@@ -55,13 +51,14 @@ def rpc(session_id, request):
         s.close()
 
 def parse_string(txt):
+    assert isinstance(txt, str)
     prefix = "<value><array><data><value>success</value><value>"
     if not txt.startswith(prefix):
-        raise "Unable to parse string response"
+        raise Exception("Unable to parse string response")
     txt = txt[len(prefix):]
     suffix = "</value></data></array></value>"
     if not txt.endswith(suffix):
-        raise "Unable to parse string response"
+        raise Exception("Unable to parse string response")
     txt = txt[:len(txt)-len(suffix)]
     return txt
 

scripts/import_file.py

@@ -0,0 +1,24 @@
+# Used for importing a non-".py" file as a module
+
+import sys
+import os
+
+def import_from_file(module_name, file_path):
+    """Import a file as a module"""
+    # Only for python3, but CI has python2 pytest check, so add this line
+    if sys.version_info.major == 2:
+        return None
+    from importlib import machinery, util
+    loader = machinery.SourceFileLoader(module_name, file_path)
+    spec = util.spec_from_loader(module_name, loader)
+    assert spec
+    assert spec.loader
+    module = util.module_from_spec(spec)
+    # Probably a good idea to add manually imported module stored in sys.modules
+    sys.modules[module_name] = module
+    spec.loader.exec_module(module)
+    return module
+
+def get_module(module_name, file_path):
+    testdir = os.path.dirname(__file__)
+    return import_from_file(module_name, testdir + file_path)

scripts/perfmon

@@ -313,7 +313,7 @@ class RRDUpdates:
         print_debug("Calling http://localhost/rrd_updates?%s" % paramstr)
 
         sock = urllib.request.urlopen("http://localhost/rrd_updates?%s" % paramstr)
-        xmlsource = sock.read()
+        xmlsource = sock.read().decode('utf-8')
         sock.close()
 
         # Use sax rather than minidom and save Vvvast amounts of time and memory.
@@ -380,7 +380,7 @@ def get_percent_mem_usage(ignored):
         memlist = memfd.readlines()
         memfd.close()
         memdict = [ m.split(':', 1) for m in memlist ]
-        memdict = dict([(k.strip(), float(re.search('\d+', v.strip()).group(0))) for (k,v) in memdict])
+        memdict = dict([(k.strip(), float(re.search(r'\d+', v.strip()).group(0))) for (k,v) in memdict])
         # We consider the sum of res memory and swap in use as the hard demand
         # of mem usage, it is bad if this number is beyond the physical mem, as
         # in such case swapping is obligatory rather than voluntary, hence
@@ -481,8 +481,12 @@ class VariableState:
     """
     def __init__(self):
         self.value = None
+        # pytype: disable=attribute-error
+        # pylint: disable-next=no-member
         self.timeof_last_alarm = time.time() - self.alarm_auto_inhibit_period
+        # pylint: disable-next=no-member
         self.trigger_down_counter = self.alarm_trigger_period
+        # pytype: enable=attribute-error
 
 class Variable(VariableConfig, VariableState):
     """ Variable() is used by ObjectMonitor to create one Variable object for each
@@ -1067,7 +1071,8 @@ def main():
             vm_uuid_list = rrd_updates.get_uuid_list_by_objtype('vm')
 
             # Remove any monitors for VMs no longer listed in rrd_updates page
-            for uuid in vm_mon_lookup:
+            # We use .pop() inside the loop, use list(dict_var.keys()):
+            for uuid in list(vm_mon_lookup.keys()):
                 if uuid not in vm_uuid_list:
                     vm_mon_lookup.pop(uuid)
 
@@ -1103,7 +1108,8 @@ def main():
             print_debug("sr_uuid_list = %s" % sr_uuid_list)
 
             # Remove monitors for SRs no longer listed in the rrd_updates page
-            for uuid in sr_mon_lookup:
+            # We use .pop() inside the loop, use list(dict_var.keys()):
+            for uuid in list(sr_mon_lookup.keys()):
                 if uuid not in sr_uuid_list:
                     sr_mon_lookup.pop(uuid)
             # Create monitors for SRs that have just appeared in rrd_updates page
@@ -1231,25 +1237,22 @@ if __name__ == "__main__":
         # we caught a signal which we have already logged
         pass
 
-    except Exception as e:
+    except Exception as e:  # pylint: disable=broad-exception-caught
         rc = 2
         log_err("FATAL ERROR: perfmon will exit")
         log_err("Exception is of class %s" % e.__class__)
         ex = sys.exc_info()
         err = traceback.format_exception(*ex)
-        
+
         # Python built-in Exception has args,
         # but XenAPI.Failure has details instead. Sigh.
         try:
-            errmsg = "\n".join([ str(x) for x in e.args ])
-            # print the exception args nicely
-            log_err(errmsg)
-        except Exception as ignored:
+            log_err("\n".join([ str(x) for x in e.args ]))
+        except AttributeError:
             try:
-                errmsg = "\n".join([ str(x) for x in e.details ])
-                # print the exception args nicely
-                log_err(errmsg)
-            except Exception as ignored:
+                assert isinstance(e, XenAPI.Failure)
+                log_err("\n".join([ str(x) for x in e.details ]))
+            except AttributeError:
                 pass
 
         # now log the traceback to syslog