Add Ruff as a code checker

Ruff is a newer code checker that is written in Rust and is very fast.
Adding this now with a number of pretty sane checks to start with.
I recommend having ruff integrated into your editor to pick out issues
as you write code.

.github/workflows/codeql-analysis.yml

@@ -39,6 +39,11 @@ jobs:
         with:
           python-version: ${{ matrix.python-version }}
 
+      - name: Run Ruff Checks
+        uses: chartboost/ruff-action@v1
+        with:
+          src: ./broker
+
       - name: Setup Temp Directory
         run: mkdir broker_dir
 

broker/__init__.py

@@ -1,3 +1,2 @@
-from broker.broker import Broker
-
-VMBroker = Broker
+"""Shortcuts for the broker module."""
+from broker.broker import Broker  # noqa: F401

broker/binds/__init__.py

@@ -0,0 +1 @@
+"""Binds provide interfaces between a provider's interface and the Broker Provider class."""

broker/binds/beaker.py

@@ -1,15 +1,18 @@
+"""A wrapper around the Beaker CLI."""
 import json
+from pathlib import Path
 import subprocess
 import time
-from logzero import logger
-from pathlib import Path
 from xml.etree import ElementTree as ET
+
+from logzero import logger
+
 from broker import helpers
 from broker.exceptions import BeakerBindError
 
 
 def _elementree_to_dict(etree):
-    """Converts an ElementTree object to a dictionary"""
+    """Convert an ElementTree object to a dictionary."""
     data = {}
     if etree.attrib:
         data.update(etree.attrib)
@@ -32,12 +35,14 @@ def _curate_job_info(job_info_dict):
         # "reservation_id": "current_reservation/recipe_id",
         "whiteboard": "whiteboard/text",
         "hostname": "recipeSet/recipe/system",
-        "distro": "recipeSet/recipe/distro"
+        "distro": "recipeSet/recipe/distro",
     }
     return helpers.dict_from_paths(job_info_dict, curated_info)
 
 
 class BeakerBind:
+    """A bind class providing a basic interface to the Beaker CLI."""
+
     def __init__(self, hub_url, auth="krbv", **kwargs):
         self.hub_url = hub_url
         self._base_args = ["--insecure", f"--hub={self.hub_url}"]
@@ -54,6 +59,11 @@ def __init__(self, hub_url, auth="krbv", **kwargs):
         self.__dict__.update(kwargs)
 
     def _exec_command(self, *cmd_args, **cmd_kwargs):
+        """Execute a beaker command and return the result.
+
+        cmd_args: Expanded into feature flags for the beaker command
+        cmd_kwargs: Expanded into args and values for the beaker command
+        """
         raise_on_error = cmd_kwargs.pop("raise_on_error", True)
         exec_cmd, cmd_args = ["bkr"], list(cmd_args)
         # check through kwargs and if any are True add to cmd_args
@@ -90,6 +100,7 @@ def _exec_command(self, *cmd_args, **cmd_kwargs):
         return result
 
     def job_submit(self, job_xml, wait=False):
+        """Submit a job to Beaker and optionally wait for it to complete."""
         # wait behavior seems buggy to me, so best to avoid it
         if not Path(job_xml).exists():
             raise FileNotFoundError(f"Job XML file {job_xml} not found")
@@ -102,36 +113,43 @@ def job_submit(self, job_xml, wait=False):
                     return line.split("'")[1].replace("J:", "")
 
     def job_watch(self, job_id):
+        """Watch a job via the job-watch command. This can be buggy."""
         job_id = f"J:{job_id}" if not job_id.startswith("J:") else job_id
         return self._exec_command("job-watch", job_id)
 
     def job_results(self, job_id, format="beaker-results-xml", pretty=False):
+        """Get the results of a job in the specified format."""
         job_id = f"J:{job_id}" if not job_id.startswith("J:") else job_id
         return self._exec_command(
             "job-results", job_id, format=format, prettyxml=pretty
         )
 
     def job_clone(self, job_id, wait=False, **kwargs):
+        """Clone a job by the specified job id."""
         job_id = f"J:{job_id}" if not job_id.startswith("J:") else job_id
         return self._exec_command("job-clone", job_id, wait=wait, **kwargs)
 
     def job_list(self, *args, **kwargs):
+        """List jobs matching the criteria specified by args and kwargs."""
         return self._exec_command("job-list", *args, **kwargs)
 
     def job_cancel(self, job_id):
+        """Cancel a job by the specified job id."""
         if not job_id.startswith("J:") and not job_id.startswith("RS:"):
             job_id = f"J:{job_id}"
         return self._exec_command("job-cancel", job_id)
 
     def job_delete(self, job_id):
+        """Delete a job by the specified job id."""
         job_id = f"J:{job_id}" if not job_id.startswith("J:") else job_id
         return self._exec_command("job-delete", job_id)
 
     def system_release(self, system_id):
+        """Release a system by the specified system id."""
         return self._exec_command("system-release", system_id)
 
     def system_list(self, **kwargs):
-        """Due to the number of arguments, we will not validate before submitting
+        """Due to the number of arguments, we will not validate before submitting.
 
         Accepted arguments are:
         available                       available to be used by this user
@@ -152,25 +170,32 @@ def system_list(self, **kwargs):
         host-filter=NAME                matching pre-defined host filter
         """
         # convert the flags passed in kwargs to arguments
-        args = []
-        for key in {"available", "free", "removed", "mine"}:
-            if kwargs.pop(key, False):
-                args.append(f"--{key}")
+        args = [
+            f"--{key}"
+            for key in ("available", "free", "removed", "mine")
+            if kwargs.pop(key, False)
+        ]
         return self._exec_command("system-list", *args, **kwargs)
 
-    def user_systems(self):  # to be used for inventory sync
-        result =  self.system_list(mine=True, raise_on_error=False)
+    def user_systems(self):
+        """Return a list of system ids owned by the current user.
+
+        This is used for inventory syncing against Beaker.
+        """
+        result = self.system_list(mine=True, raise_on_error=False)
         if result.status != 0:
             return []
         else:
             return result.stdout.splitlines()
 
     def system_details(self, system_id, format="json"):
+        """Get details about a system by the specified system id."""
         return self._exec_command("system-details", system_id, format=format)
 
     def execute_job(self, job, max_wait="24h"):
-        """Submit a job, periodically checking the status until it completes
-        then return a dictionary of the results.
+        """Submit a job, periodically checking the status until it completes.
+
+        return: a dictionary of the results.
         """
         if Path(job).exists():  # job xml path passed in
             job_id = self.job_submit(job, wait=False)
@@ -182,15 +207,20 @@ def execute_job(self, job, max_wait="24h"):
             time.sleep(60)
             result = self.job_results(job_id, pretty=True)
             if 'result="Pass"' in result.stdout:
-                return _curate_job_info(_elementree_to_dict(ET.fromstring(result.stdout)))
+                return _curate_job_info(
+                    _elementree_to_dict(ET.fromstring(result.stdout))
+                )
             elif 'result="Fail"' in result.stdout or "Exception: " in result.stdout:
                 raise BeakerBindError(f"Job {job_id} failed:\n{result}")
             elif 'result="Warn"' in result.stdout:
                 res_dict = _elementree_to_dict(ET.fromstring(result.stdout))
-                raise BeakerBindError(f"Job {job_id} was resulted in a warning. Status: {res_dict['status']}")
+                raise BeakerBindError(
+                    f"Job {job_id} was resulted in a warning. Status: {res_dict['status']}"
+                )
         raise BeakerBindError(f"Job {job_id} did not complete within {max_wait}")
 
     def system_details_curated(self, system_id):
+        """Return a curated dictionary of system details."""
         full_details = json.loads(self.system_details(system_id).stdout)
         curated_details = {
             "hostname": full_details["fqdn"],
@@ -216,9 +246,11 @@ def system_details_curated(self, system_id):
         return curated_details
 
     def jobid_from_system(self, system_hostname):
-        """Return the job id for the current reservation on the system"""
+        """Return the job id for the current reservation on the system."""
         for job_id in json.loads(self.job_list(mine=True).stdout):
             job_result = self.job_results(job_id, pretty=True)
-            job_detail = _curate_job_info(_elementree_to_dict(ET.fromstring(job_result.stdout)))
+            job_detail = _curate_job_info(
+                _elementree_to_dict(ET.fromstring(job_result.stdout))
+            )
             if job_detail["hostname"] == system_hostname:
                 return job_id

broker/binds/containers.py

@@ -1,4 +1,9 @@
+"""A collection of classes to ease interaction with Docker and Podman libraries."""
+
+
 class ContainerBind:
+    """A base class that provides common functionality for Docker and Podman containers."""
+
     _sensitive_attrs = ["password", "host_password"]
 
     def __init__(self, host=None, username=None, password=None, port=22, timeout=None):
@@ -12,19 +17,23 @@ def __init__(self, host=None, username=None, password=None, port=22, timeout=Non
 
     @property
     def client(self):
+        """Return the client instance. Create one if it does not exist."""
         if not isinstance(self._client, self._ClientClass):
             self._client = self._ClientClass(base_url=self.uri, timeout=self.timeout)
         return self._client
 
     @property
     def images(self):
+        """Return a list of images on the container host."""
         return self.client.images.list()
 
     @property
     def containers(self):
+        """Return a list of containers on the container host."""
         return self.client.containers.list(all=True)
 
     def image_info(self, name):
+        """Return curated information about an image on the container host."""
         if image := self.client.images.get(name):
             return {
                 "id": image.short_id,
@@ -36,29 +45,33 @@ def image_info(self, name):
             }
 
     def create_container(self, image, command=None, **kwargs):
-        """Create and return running container instance"""
+        """Create and return running container instance."""
         kwargs = self._sanitize_create_args(kwargs)
         return self.client.containers.create(image, command, **kwargs)
 
     def execute(self, image, command=None, remove=True, **kwargs):
-        """Run a container and return the raw result"""
+        """Run a container and return the raw result."""
         return self.client.containers.run(
             image, command=command, remove=remove, **kwargs
         ).decode()
 
     def remove_container(self, container=None):
+        """Remove a container from the container host."""
         if container:
             container.remove(v=True, force=True)
 
     def pull_image(self, name):
+        """Pull an image into the container host."""
         return self.client.images.pull(name)
 
     @staticmethod
     def get_logs(container):
-        return "\n".join(map(lambda x: x.decode(), container.logs(stream=False)))
+        """Return the logs from a container."""
+        return "\n".join(x.decode() for x in container.logs(stream=False))
 
     @staticmethod
     def get_attrs(cont):
+        """Return curated information about a container."""
         return {
             "id": cont.id,
             "image": cont.attrs.get("ImageName", cont.attrs["Image"]),
@@ -69,6 +82,7 @@ def get_attrs(cont):
         }
 
     def __repr__(self):
+        """Return a string representation of the object."""
         inner = ", ".join(
             f"{k}={'******' if k in self._sensitive_attrs and v else v}"
             for k, v in self.__dict__.items()
@@ -78,6 +92,8 @@ def __repr__(self):
 
 
 class PodmanBind(ContainerBind):
+    """Handles Podman-specific connection and implementation differences."""
+
     def __init__(self, **kwargs):
         super().__init__(**kwargs)
         from podman import PodmanClient
@@ -94,16 +110,24 @@ def __init__(self, **kwargs):
 
     def _sanitize_create_args(self, kwargs):
         from podman.domain.containers_create import CreateMixin
+
         try:
             CreateMixin._render_payload(kwargs)
         except TypeError as err:
-            sanitized = err.args[0].replace("Unknown keyword argument(s): ", "").replace("'", "").split(" ,")
+            sanitized = (
+                err.args[0]
+                .replace("Unknown keyword argument(s): ", "")
+                .replace("'", "")
+                .split(" ,")
+            )
             kwargs = {k: v for k, v in kwargs.items() if k not in sanitized}
             kwargs = self._sanitize_create_args(kwargs)
         return kwargs
 
 
 class DockerBind(ContainerBind):
+    """Handles Docker-specific connection and implementation differences."""
+
     def __init__(self, port=2375, **kwargs):
         kwargs["port"] = port
         super().__init__(**kwargs)
@@ -117,4 +141,5 @@ def __init__(self, port=2375, **kwargs):
 
     def _sanitize_create_args(self, kwargs):
         from docker.models.containers import RUN_CREATE_KWARGS
+
         return {k: v for k, v in kwargs.items() if k in RUN_CREATE_KWARGS}