Merge branch 'manager-nits' into '6.0'

manager: nits, cleanups, fixes

See merge request knot/knot-resolver!1496

Author: Oto Šťáva

manager/.python-version

@@ -1,5 +1,5 @@
-3.7.17
-3.8.17
-3.9.17
+3.11.7
 3.10.12
-3.11.4
+3.9.17
+3.8.17
+3.7.17

manager/build.py manager/build_c_extensions.py

@@ -58,7 +58,7 @@ def get_socket_from_config(config: Path, optional_file: bool) -> Optional[Socket
                 )
         return None
     except ValueError as e:
-        raise DataValidationError(*e.args)
+        raise DataValidationError(*e.args) from e
     except OSError as e:
         if not optional_file:
             raise e

manager/knot_resolver_manager/cli/command.py

@@ -2,15 +2,6 @@
 #
 # pylint: disable=no-member
 
-# We disable pyright checks because it can't find method that don't exist in this Python version
-# so the reported error is correct, but due to the version checking conditions, it never happens.
-# Due to backporting, we are also using private methods and non-existent members of classes
-#
-# pyright: reportUnknownMemberType=false
-# pyright: reportUnknownVariableType=false
-# pyright: reportGeneralTypeIssues=false
-# pyright: reportPrivateUsage=false
-
 import asyncio
 import functools
 import logging

manager/knot_resolver_manager/compat/asyncio.py

@@ -159,7 +159,7 @@ def render_lua(self) -> str:
         # FIXME the `cwd` argument is used only for configuring control socket path
         # it should be removed and relative path used instead as soon as issue
         # https://gitlab.nic.cz/knot/knot-resolver/-/issues/720 is fixed
-        return MAIN_TEMPLATE.render(cfg=self, cwd=os.getcwd())  # pyright: reportUnknownMemberType=false
+        return MAIN_TEMPLATE.render(cfg=self, cwd=os.getcwd())
 
 
 def get_rundir_without_validation(data: Dict[str, Any]) -> Dir:

manager/knot_resolver_manager/datamodel/config_schema.py

@@ -147,11 +147,11 @@ def __init__(self, source_value: Any, object_path: str = "/") -> None:
         super().__init__(source_value, object_path)
         try:
             punycode = self._value.encode("idna").decode("utf-8") if self._value != "." else "."
-        except ValueError:
+        except ValueError as e:
             raise ValueError(
                 f"conversion of '{self._value}' to IDN punycode representation failed",
                 object_path,
-            )
+            ) from e
 
         if type(self)._re.match(punycode):
             self._punycode = punycode

manager/knot_resolver_manager/datamodel/types/types.py

@@ -32,7 +32,7 @@ class SupervisordKresID(KresID):
 
     @staticmethod
     def from_string(val: str) -> "SupervisordKresID":
-        if val == "cache-gc" or val == "cache-gc:cache-gc":
+        if val in ("cache-gc", "cache-gc:cache-gc"):
             # the double name is checked because thats how we read it from supervisord
             return SupervisordKresID.new(SubprocessType.GC, 0)
         else:
@@ -170,7 +170,7 @@ async def write_config_file(config: KresConfig) -> None:
     template = await read_resource(__package__, "supervisord.conf.j2")
     assert template is not None
     template = template.decode("utf8")
-    config_string = Template(template).render(  # pyright: reportUnknownMemberType=false
+    config_string = Template(template).render(
         gc=ProcessTypeConfig.create_gc_config(config),
         kresd=ProcessTypeConfig.create_kresd_config(config),
         manager=ProcessTypeConfig.create_manager_config(config),

manager/knot_resolver_manager/kresd_controller/supervisord/config_file.py

@@ -432,7 +432,7 @@ def _set_working_directory(config_raw: Dict[str, Any]) -> None:
     try:
         rundir = get_rundir_without_validation(config_raw)
     except ValueError as e:
-        raise DataValidationError(str(e), "/rundir")
+        raise DataValidationError(str(e), "/rundir") from e
 
     logger.debug(f"Changing working directory to '{rundir.to_path().absolute()}'.")
     os.chdir(rundir.to_path())
@@ -457,12 +457,12 @@ def _lock_working_directory(attempt: int = 0) -> None:
             raise KresManagerException(
                 "Another manager is running in the same working directory."
                 f" PID file is located at {os.getcwd()}/{PID_FILE_NAME}"
-            )
+            ) from e
         else:
             raise KresManagerException(
                 "Another manager is running in the same working directory."
                 f" PID file is located at {os.getcwd()}/{PID_FILE_NAME}"
-            )
+            ) from e
 
     # now we know that we are the only manager running in this directory
 

manager/knot_resolver_manager/server.py

@@ -22,10 +22,10 @@ def f(*nargs: Any, **nkwargs: Any) -> Optional[T]:
             try:
                 return func(*nargs, **nkwargs)
             except BaseException as e:
-                if isinstance(e, exceptions):  # pyright: reportUnnecessaryIsInstance=false
+                if isinstance(e, exceptions):
                     return default
                 else:
-                    raise e  # pyright: reportGeneralTypeIssues=false
+                    raise e
 
         return f
 

manager/knot_resolver_manager/utils/__init__.py

@@ -521,7 +521,7 @@ def _assign_default(self, obj: Any, name: str, python_type: Any, object_path: st
         try:
             default = self._create_default(getattr(cls, name, None))
         except ValueError as e:
-            raise DataValidationError(str(e), f"{object_path}/{name}")
+            raise DataValidationError(str(e), f"{object_path}/{name}") from e
 
         value = self.map_object(python_type, default, object_path=f"{object_path}/{name}")
         setattr(obj, name, value)

manager/knot_resolver_manager/utils/modeling/base_schema.py

@@ -69,10 +69,10 @@ def resolve(
                         try:
                             token = int(token)
                             current = current[token]
-                        except ValueError:
+                        except ValueError as e:
                             raise ValueError(
                                 f"invalid JSON pointer: list '{current_ptr}' require numbers as keys, instead got '{token}'"
-                            )
+                            ) from e
 
                 elif isinstance(current, dict):
                     current = current.get(token, None)

manager/knot_resolver_manager/utils/modeling/json_pointer.py

@@ -39,7 +39,7 @@ def construct_mapping(self, node: Union[MappingNode, Any], deep: bool = False) -
                     node.start_mark,
                     f"found unacceptable key ({exc})",
                     key_node.start_mark,
-                )
+                ) from exc
 
             # check for duplicate keys
             if key in mapping:
@@ -92,4 +92,8 @@ def try_to_parse(data: str) -> Any:
         try:
             return parse_yaml(data)
         except yaml.YAMLError as ye:
-            raise DataParsingError(f"failed to parse data, JSON: {je}, YAML: {ye}")
+            # We do not raise-from here because there are two possible causes
+            # and we may not know which one is the actual one.
+            raise DataParsingError(  # pylint: disable=raise-missing-from
+                f"failed to parse data, JSON: {je}, YAML: {ye}"
+            )

manager/knot_resolver_manager/utils/modeling/parsing.py

@@ -10,7 +10,7 @@ authors = [
 # See currently open issue about building C extensions here:
 # https://github.com/python-poetry/poetry/issues/2740
 [tool.poetry.build]
-script = "build.py"
+script = "build_c_extensions.py"
 generate-setup-file = true
 
 [tool.poetry.dependencies]
@@ -64,7 +64,7 @@ run-debug = { cmd = "scripts/run-debug", help = "Run the manager under debugger"
 docs = { cmd = "scripts/docs", help = "Create HTML documentation" }
 test = { shell = "env PYTHONPATH=. pytest --junitxml=unit.junit.xml --cov=knot_resolver_manager --show-capture=all tests/unit/", help = "Run tests" }
 check = { cmd = "scripts/codecheck", help = "Run static code analysis" }
-format = { shell = "black knot_resolver_manager/ tests/ scripts/ build.py; isort -rc .", help = "Run code formatter" }
+format = { shell = "black knot_resolver_manager/ tests/ scripts/ build_c_extensions.py; isort -rc .", help = "Run code formatter" }
 fixdeps = { shell = "poetry install; npm install; npm update", help = "Install/update dependencies according to configuration files"}
 commit = { shell = "scripts/commit", help = "Invoke every single check before commiting" }
 container = { cmd = "scripts/container.py", help = "Manage containers" }
@@ -129,22 +129,16 @@ disable= [
     "missing-docstring",
     "no-else-return",
     "no-self-use",
-    "raise-missing-from",
     "too-few-public-methods",
     "unused-import",  # checked by flake8,
-    "bad-continuation", # conflicts with black
-    "consider-using-in", # pyright can't see through in expressions,
     "too-many-return-statements", # would prevent us from using recursive tree traversals
     "logging-fstring-interpolation", # see https://github.com/PyCQA/pylint/issues/1788
     "no-else-raise", # not helpful for readability, when we want explicit branches
     "raising-bad-type", # handled by type checker
     "too-many-arguments",  # sure, but how can we change the signatures to take less arguments? artificially create objects with arguments? That's stupid...
-    "no-member",  # checked by pyright
-    "import-error", # checked by pyright (and pylint does not do it properly)
-    "unsupported-delete-operation", # checked by pyright
-    "unsubscriptable-object", # checked by pyright
-    "unsupported-membership-test", # checked by pyright
     "invalid-overridden-method",  # hopefully checked by type checkers
+    "no-member",  # pylint does not do it properly
+    "import-error", # pylint does not do it properly
 ]
 
 [tool.pylint.SIMILARITIES]
@@ -156,14 +150,6 @@ ignore-imports = "yes"
 [tool.pylint.DESIGN]
 max-parents = "10"
 
-[tool.pyright]
-include = [
-    "knot_resolver_manager",
-    "tests"
-]
-exclude = []
-typeCheckingMode = "strict"
-
 [tool.mypy]
 python_version = "3.7"
 # strict = true

manager/pyproject.toml

@@ -46,7 +46,7 @@
     'entry_points': entry_points,
     'python_requires': '>=3.7,<4.0',
 }
-from build import *
+from build_c_extensions import *
 build(setup_kwargs)
 
 setup(**setup_kwargs)

manager/setup.py

@@ -12,7 +12,7 @@
 dummy.__dict__["setup"] = lambda *args, **kwargs: None
 dummy.__dict__["build"] = lambda *args, **kwargs: None
 sys.modules["setuptools"] = dummy
-sys.modules["build"] = dummy
+sys.modules["build_c_extensions"] = dummy
 
 # load install_requires array from setup.py
 spec = importlib.util.spec_from_file_location("setup", sys.argv[1] if len(sys.argv) == 2 else "manager/setup.py")