NO-JIRA: chore(ruff): fix indentation and linting in the checked code so that ruff passes (#1009)

* NO-JIRA: chore(ruff): fix indentation and linting in the checked code so that ruff passes

* apply ruff autofixes

* apply ruff --unsafe-fixes

* G004 Logging statement uses f-string

* UP035 `typing.ContextManager` is deprecated, use `contextlib.AbstractContextManager` instead

* PLE0604 Invalid object in `__all__`, must contain only strings

* N815 Variable `volumePath` in class scope should not be mixedCase

* PLE0604 Invalid object in `__all__`, must contain only strings

* N818 Exception name `WaitException` should be named with an Error suffix

* B904 Within an `except` clause, raise exceptions with `raise ... from err` or `raise ... from None` to distinguish them from errors in exception handling

* # ruff: noqa: PLC0415 `import` should be at the top-level of a file

* PLE0604 Invalid object in `__all__`, must contain only strings

* B008 Do not perform function call `os.getcwd` in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

* PLR0911 Too many return statements (9 > 6)

* N801 Class name `ANNOTATION_TYPE` should use CapWords convention

* PLR0914 Too many local variables (19/15)

* PLE0604 Invalid object in `__all__`, must contain only strings

* RUF013 PEP 484 prohibits implicit `Optional`

* PYI034 `__enter__` methods in classes like `TestFrame` usually return `self` at runtime

* PLC2801 Unnecessary dunder call to `__enter__`

* N803 Argument name `HASH_N` should be lowercase

* suppress what remains

* undo unreadable reformat

Author: jiridanek

.editorconfig

@@ -10,3 +10,6 @@ trim_trailing_whitespace = true
 
 [Makefile]
 indent_style = tab
+
+[*.py]
+max_line_length = 120

.pre-commit-config.yaml

@@ -6,18 +6,18 @@ repos:
     rev: 0.6.13
     hooks:
       - id: uv-lock
-      #  # https://github.com/astral-sh/ruff-pre-commit
-      #  - repo: https://github.com/astral-sh/ruff-pre-commit
-      #    rev: v0.11.4
-      #    hooks:
-      #      - id: ruff
-      #        types_or: [python, pyi]
-      #        args: [ --fix ]
-      #        files: 'ci/.*|tests/.*'
-      #      - id: ruff-format
-      #        types_or: [python, pyi]
-      #        files: 'ci/.*|tests/.*'
-      #  # https://pre-commit.com/#new-hooks
+  # https://github.com/astral-sh/ruff-pre-commit
+  - repo: https://github.com/astral-sh/ruff-pre-commit
+    rev: v0.11.4
+    hooks:
+      - id: ruff
+        types_or: [python, pyi]
+        args: [--fix]
+        files: 'ci/.*|tests/.*'
+      - id: ruff-format
+        types_or: [python, pyi]
+        files: 'ci/.*|tests/.*'
+  # https://pre-commit.com/#new-hooks
   - repo: local
     hooks:
       # https://github.com/microsoft/pyright/issues/3612

ci/cached-builds/gen_gha_matrix_jobs.py

@@ -4,11 +4,11 @@
 import enum
 import json
 import logging
-import platform
-import subprocess
 import os
 import pathlib
+import platform
 import re
+import subprocess
 import sys
 import unittest
 
@@ -25,36 +25,43 @@
 
 def parse_makefile(target: str, makefile_dir: pathlib.Path | str) -> str:
     # Check if the operating system is macOS
-    if platform.system() == 'Darwin':
-        make_command = 'gmake'
+    if platform.system() == "Darwin":
+        make_command = "gmake"
     else:
-        make_command = 'make'
+        make_command = "make"
 
     try:
         # Run the make (or gmake) command and capture the output
-        result = subprocess.run([make_command, '-nps', target],
-                                stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True,
-                                check=True, cwd=makefile_dir)
+        result = subprocess.run(
+            [make_command, "-nps", target],
+            capture_output=True,
+            text=True,
+            check=True,
+            cwd=makefile_dir,
+        )
     except subprocess.CalledProcessError as e:
         # Handle errors if the make command fails
-        print(f'{make_command} failed with return code: {e.returncode}:\n{e.stderr}', file=sys.stderr)
+        print(f"{make_command} failed with return code: {e.returncode}:\n{e.stderr}", file=sys.stderr)
         raise
     except Exception as e:
         # Handle any other exceptions
-        print(f'Error occurred attempting to parse Makefile:\n{str(e)}', file=sys.stderr)
+        print(f"Error occurred attempting to parse Makefile:\n{e!s}", file=sys.stderr)
         raise
 
     return result.stdout
 
 
-def extract_image_targets(makefile_dir: pathlib.Path | str = os.getcwd()) -> list[str]:
-    makefile_all_target = 'all-images'
+def extract_image_targets(makefile_dir: pathlib.Path | str | None = None) -> list[str]:
+    if makefile_dir is None:
+        makefile_dir = os.getcwd()
+
+    makefile_all_target = "all-images"
 
     output = parse_makefile(target=makefile_all_target, makefile_dir=makefile_dir)
 
     # Extract the 'all-images' entry and its values
     all_images = []
-    match = re.search(rf'^{makefile_all_target}:\s+(.*)$', output, re.MULTILINE)
+    match = re.search(rf"^{makefile_all_target}:\s+(.*)$", output, re.MULTILINE)
     if match:
         all_images = match.group(1).split()
 
@@ -74,18 +81,26 @@ def main() -> None:
     logging.basicConfig(level=logging.DEBUG, stream=sys.stderr)
 
     argparser = argparse.ArgumentParser()
-    argparser.add_argument("--from-ref", type=str, required=False,
-                           help="Git ref of the base branch (to determine changed files)")
-    argparser.add_argument("--to-ref", type=str, required=False,
-                           help="Git ref of the PR branch (to determine changed files)")
-    argparser.add_argument("--rhel-images", type=RhelImages, required=False, default=RhelImages.INCLUDE, nargs='?',
-                           help="Whether to `include` rhel images or `exclude` them or `include-only` them")
+    argparser.add_argument(
+        "--from-ref", type=str, required=False, help="Git ref of the base branch (to determine changed files)"
+    )
+    argparser.add_argument(
+        "--to-ref", type=str, required=False, help="Git ref of the PR branch (to determine changed files)"
+    )
+    argparser.add_argument(
+        "--rhel-images",
+        type=RhelImages,
+        required=False,
+        default=RhelImages.INCLUDE,
+        nargs="?",
+        help="Whether to `include` rhel images or `exclude` them or `include-only` them",
+    )
     args = argparser.parse_args()
 
     targets = extract_image_targets()
 
     if args.from_ref:
-        logging.info(f"Skipping targets not modified in the PR")
+        logging.info("Skipping targets not modified in the PR")
         changed_files = gha_pr_changed_files.list_changed_files(args.from_ref, args.to_ref)
         targets = gha_pr_changed_files.filter_out_unchanged(targets, changed_files)
 
@@ -100,14 +115,14 @@ def main() -> None:
 
     # https://stackoverflow.com/questions/66025220/paired-values-in-github-actions-matrix
     output = [
-        "matrix=" + json.dumps({
-            "include": [
-                {"target": target, "subscription": "rhel" in target} for target in targets
-            ],
-        }, separators=(',', ':')),
-        "has_jobs=" + json.dumps(
-            len(targets) > 0, separators=(',', ':')
+        "matrix="
+        + json.dumps(
+            {
+                "include": [{"target": target, "subscription": "rhel" in target} for target in targets],
+            },
+            separators=(",", ":"),
         ),
+        "has_jobs=" + json.dumps(len(targets) > 0, separators=(",", ":")),
     ]
 
     print("targets", targets)
@@ -118,10 +133,10 @@ def main() -> None:
             for entry in output:
                 print(entry, file=f)
     else:
-        logging.info(f"Not running on Github Actions, won't produce GITHUB_OUTPUT")
+        logging.info("Not running on Github Actions, won't produce GITHUB_OUTPUT")
 
 
-if __name__ == '__main__':
+if __name__ == "__main__":
     main()
 
 
@@ -132,7 +147,7 @@ def test_select_changed_targets_dockerfile(self):
         changed_files = ["jupyter/datascience/ubi9-python-3.11/Dockerfile.cpu"]
 
         targets = gha_pr_changed_files.filter_out_unchanged(targets, changed_files)
-        assert set(targets) == {'jupyter-datascience-ubi9-python-3.11'}
+        assert set(targets) == {"jupyter-datascience-ubi9-python-3.11"}
 
     def test_select_changed_targets_shared_file(self):
         targets = extract_image_targets(makefile_dir=project_dir)
@@ -145,10 +160,12 @@ def test_select_changed_targets_shared_file(self):
         # also being returned.  Odds of this inefficiency noticably "hurting us" is low - so of the opinion we can
         # simply treat this as technical debt.
         targets = gha_pr_changed_files.filter_out_unchanged(targets, changed_files)
-        assert set(targets) == {'jupyter-minimal-ubi9-python-3.11',
-                                'cuda-jupyter-minimal-ubi9-python-3.11',
-                                'cuda-jupyter-pytorch-ubi9-python-3.11',
-                                'runtime-cuda-pytorch-ubi9-python-3.11',
-                                'cuda-jupyter-tensorflow-ubi9-python-3.11',
-                                'rocm-jupyter-minimal-ubi9-python-3.11',
-                                'runtime-cuda-tensorflow-ubi9-python-3.11'}
+        assert set(targets) == {
+            "jupyter-minimal-ubi9-python-3.11",
+            "cuda-jupyter-minimal-ubi9-python-3.11",
+            "cuda-jupyter-pytorch-ubi9-python-3.11",
+            "runtime-cuda-pytorch-ubi9-python-3.11",
+            "cuda-jupyter-tensorflow-ubi9-python-3.11",
+            "rocm-jupyter-minimal-ubi9-python-3.11",
+            "runtime-cuda-tensorflow-ubi9-python-3.11",
+        }

ci/cached-builds/gha_pr_changed_files.py

@@ -1,7 +1,7 @@
+import fnmatch
 import json
 import logging
 import os
-import fnmatch
 import pathlib
 import re
 import shutil
@@ -13,16 +13,17 @@
 
 
 def get_github_token() -> str:
-    github_token = os.environ['GITHUB_TOKEN']
+    github_token = os.environ["GITHUB_TOKEN"]
     return github_token
 
 
 def list_changed_files(from_ref: str, to_ref: str) -> list[str]:
     logging.debug("Getting list of changed files from git diff")
 
     # https://github.com/red-hat-data-services/notebooks/pull/361: add -- in case to_ref matches a file name in the repo
-    files = subprocess.check_output(["git", "diff", "--name-only", from_ref, to_ref, '--'],
-                                    encoding='utf-8').splitlines()
+    files = subprocess.check_output(
+        ["git", "diff", "--name-only", from_ref, to_ref, "--"], encoding="utf-8"
+    ).splitlines()
 
     logging.debug(f"Determined {len(files)} changed files: {files[:100]} (..., printing up to 100 files)")
     return files
@@ -34,8 +35,9 @@ def _query_build(make_target: str, query: str) -> str:
     pattern = re.compile(r"#\*# " + query + r": <(?P<result>[^>]+)> #\(MACHINE-PARSED LINE\)#\*#\.\.\.")
     try:
         logging.debug(f"Running make in --just-print mode for target {make_target}")
-        for line in subprocess.check_output([MAKE, make_target, "--just-print"], encoding="utf-8",
-                                            cwd=PROJECT_ROOT).splitlines():
+        for line in subprocess.check_output(
+            [MAKE, make_target, "--just-print"], encoding="utf-8", cwd=PROJECT_ROOT
+        ).splitlines():
             if m := pattern.match(line):
                 results.append(m["result"])
     except subprocess.CalledProcessError as e:
@@ -61,7 +63,7 @@ def find_dockerfiles(directory: str) -> list:
     """Finds and returns a list of files matching the pattern 'Dockerfile*' in the specified directory."""
     matching_files = []
     for filename in os.listdir(directory):
-        if fnmatch.fnmatch(filename, 'Dockerfile*') and filename != 'Dockerfile.konflux':
+        if fnmatch.fnmatch(filename, "Dockerfile*") and filename != "Dockerfile.konflux":
             matching_files.append(filename)
     return matching_files
 
@@ -77,8 +79,9 @@ def should_build_target(changed_files: list[str], target_directory: str) -> str:
     # detect change in any of the files outside
     dockerfiles = find_dockerfiles(target_directory)
     for dockerfile in dockerfiles:
-        stdout = subprocess.check_output([PROJECT_ROOT / "bin/buildinputs", target_directory + "/" + dockerfile],
-                                         text=True, cwd=PROJECT_ROOT)
+        stdout = subprocess.check_output(
+            [PROJECT_ROOT / "bin/buildinputs", target_directory + "/" + dockerfile], text=True, cwd=PROJECT_ROOT
+        )
         logging.debug(f"{target_directory=} {dockerfile=} {stdout=}")
         if stdout == "\n":
             # no dependencies
@@ -107,8 +110,10 @@ class SelfTests(unittest.TestCase):
     def test_list_changed_files(self):
         """This is PR #556 in opendatahub-io/notebooks"""
         changed_files = list_changed_files(from_ref="4d4841f", to_ref="2c36c11")
-        assert set(changed_files) == {'codeserver/ubi9-python-3.9/Dockerfile',
-                                      'codeserver/ubi9-python-3.9/run-code-server.sh'}
+        assert set(changed_files) == {
+            "codeserver/ubi9-python-3.9/Dockerfile",
+            "codeserver/ubi9-python-3.9/run-code-server.sh",
+        }
 
     def test_get_build_directory(self):
         directory = get_build_directory("rocm-jupyter-pytorch-ubi9-python-3.11")

ci/cached-builds/has_tests.py

@@ -16,13 +16,14 @@
 
 class Args(argparse.Namespace):
     """Type annotation to have autocompletion for args"""
+
     target: str
 
 
 def main() -> None:
     parser = argparse.ArgumentParser("make_test.py")
     parser.add_argument("--target", type=str)
-    args = typing.cast(Args, parser.parse_args())
+    args = typing.cast("Args", parser.parse_args())
 
     has_tests = check_tests(args.target)
 
@@ -34,11 +35,13 @@ def main() -> None:
 
 
 def check_tests(target: str) -> bool:
-    if target.startswith("rocm-jupyter-minimal-") or target.startswith("rocm-jupyter-datascience-"):
+    if target.startswith(("rocm-jupyter-minimal-", "rocm-jupyter-datascience-")):
         return False  # we don't have specific tests for -minimal-, ... in ci-operator/config
 
     build_directory = gha_pr_changed_files.get_build_directory(target)
-    kustomization = pathlib.Path(gha_pr_changed_files.PROJECT_ROOT) / build_directory / "kustomize/base/kustomization.yaml"
+    kustomization = (
+        pathlib.Path(gha_pr_changed_files.PROJECT_ROOT) / build_directory / "kustomize/base/kustomization.yaml"
+    )
 
     return kustomization.is_file()