Merge pull request #765 from nolar/refactor-layers-3

Rebalance the dependency tree by moving & tossing the modules

Author: nolar

.github/workflows/ci.yaml

@@ -20,6 +20,7 @@ jobs:
         with:
           python-version: "3.9"
       - run: pip install -r requirements.txt
+      - run: lint-imports
       - run: isort . --check --diff
         continue-on-error: true
       - run: isort examples --settings=examples --check --diff

.github/workflows/thorough.yaml

@@ -24,6 +24,7 @@ jobs:
         with:
           python-version: "3.9"
       - run: pip install -r requirements.txt
+      - run: lint-imports
       - run: isort . --check --diff
         continue-on-error: true
       - run: isort examples --settings=examples --check --diff

.importlinter

@@ -0,0 +1,110 @@
+; Importing constraints for layered layout of modules and packages.
+; The goal is higher cohesion and lower coupling of components.
+; https://import-linter.readthedocs.io/en/stable/contract_types.html
+[importlinter]
+root_package = kopf
+include_external_packages = True
+contract_types =
+    conditional: _importlinter_conditional.ConditionalImportContract
+
+[importlinter:contract:root-layers]
+name = The root framework modules must be layered
+type = layers
+layers =
+    kopf.on
+    kopf._kits
+    kopf._core
+    kopf._cogs
+
+[importlinter:contract:core-layers]
+name = The internal core must be layered
+type = layers
+layers =
+    kopf._core.reactor
+    kopf._core.engines
+    kopf._core.intents
+    kopf._core.actions
+
+[importlinter:contract:cogs-layers]
+name = The internal cogs must be layered
+type = layers
+layers =
+    kopf._cogs.clients
+    kopf._cogs.configs
+    kopf._cogs.structs
+    kopf._cogs.aiokits
+    kopf._cogs.helpers
+
+[importlinter:contract:progress-storage]
+name = Progress storages must be persistence settings
+type = layers
+layers =
+    kopf._cogs.configs.configuration
+    kopf._cogs.configs.progress
+    kopf._cogs.configs.conventions
+
+[importlinter:contract:diffbase-storage]
+name = Diffbase storages must be persistence settings
+type = layers
+layers =
+    kopf._cogs.configs.configuration
+    kopf._cogs.configs.diffbase
+    kopf._cogs.configs.conventions
+
+[importlinter:contract:independent-storages]
+name = Storage types must be unaware of each other
+type = independence
+modules =
+    kopf._cogs.configs.diffbase
+    kopf._cogs.configs.progress
+
+[importlinter:contract:independent-aiokits]
+name = Most asyncio kits must be unaware of each other
+type = independence
+modules =
+    kopf._cogs.aiokits.aioadapters
+    kopf._cogs.aiokits.aiobindings
+    kopf._cogs.aiokits.aioenums
+    kopf._cogs.aiokits.aiotoggles
+    kopf._cogs.aiokits.aiovalues
+    ; but not aiotasks & aiotime!
+
+[importlinter:contract:ban-toolkits]
+name = The internals must be unaware of user-facing toolkits
+type = forbidden
+source_modules =
+    kopf._cogs
+    kopf._core
+forbidden_modules =
+    kopf._kits
+
+[importlinter:contract:indenpendent-toolkits]
+name = The user-facing toolkits must be unaware of each other
+type = independence
+modules =
+    kopf._kits.hierarchies
+    kopf._kits.runner
+    kopf._kits.webhooks
+
+[importlinter:contract:allow-3rd-party]
+name = 3rd-party clients must be explicitly allowed
+type = forbidden
+source_modules =
+    kopf
+forbidden_modules =
+    pykube
+    kubernetes
+ignore_imports =
+    kopf._core.intents.piggybacking -> pykube
+    kopf._core.intents.piggybacking -> kubernetes
+    kopf._cogs.helpers.thirdparty -> pykube
+    kopf._cogs.helpers.thirdparty -> kubernetes
+
+[importlinter:contract:secure-3rd-party]
+name = 3rd-party clients must be secured by conditional imports
+type = conditional
+source_modules =
+    kopf
+conditional_modules =
+    pykube
+    kubernetes

_importlinter_conditional.py

@@ -0,0 +1,92 @@
+"""
+A contract for the import linter to secure 3rd-party clients importing.
+
+Wrong::
+
+    import kubernetes
+
+Right::
+
+    try:
+        import kubernetes
+    except ImportError:
+        ...
+
+https://import-linter.readthedocs.io/en/stable/custom_contract_types.html
+"""
+import os.path
+
+import astpath
+from importlinter import Contract, ContractCheck, fields, output
+
+
+class ConditionalImportContract(Contract):
+    """
+    Contract that defines a single forbidden import between
+    two modules.
+    """
+    source_modules = fields.ListField(subfield=fields.ModuleField())
+    conditional_modules = fields.ListField(subfield=fields.ModuleField())
+
+    def check(self, graph):
+        failed_details = []
+
+        # Combine all source x all target (secured) modules.
+        conditional_modules = [m for m in self.conditional_modules if m.name in graph.modules]
+        for source_module in self.source_modules:
+            for conditional_module in conditional_modules:
+
+                # For every pair of source & target, find all import chains.
+                chains = graph.find_shortest_chains(
+                    importer=source_module.name,
+                    imported=conditional_module.name,
+                )
+                for chain in chains:
+                    # Of each chain, we only need the tail for our analysis.
+                    # A sample chain: ('kopf.on', 'kopf._core.intents.registries', 'pykube')
+                    importer, imported = chain[-2:]
+                    details = graph.get_import_details(
+                        importer=importer,
+                        imported=imported
+                    )
+
+                    # For each import (possible several per file), get its line number and check it.
+                    for detail in details:
+                        ok = self._check_secured_import(detail['importer'], detail['line_number'])
+                        if not ok:
+                            failed_details.append(detail)
+
+        return ContractCheck(
+            kept=not failed_details,
+            metadata={'failed_details': failed_details},
+        )
+
+    def render_broken_contract(self, check):
+        for detail in check.metadata['failed_details']:
+            importer = detail['importer']
+            imported = detail['imported']
+            line_number = detail['line_number']
+            line_contents = detail['line_contents']
+            output.print_error(
+                f'{importer} is not allowed to import {imported} without try-except-ImportError:',
+                bold=True,
+            )
+            output.new_line()
+            output.indent_cursor()
+            output.print_error(f'{importer}:{line_number}: {line_contents}')
+
+    def _check_secured_import(self, mod: str, lno: int) -> bool:
+
+        # Some hard-coded heuristics because importlib fails on circular imports.
+        # TODO: switch to: importlib.util.find_spec(mod)?.origin
+        path = os.path.join(os.path.dirname(__file__), mod.replace('.', '/')) + '.py'
+        with open(path, 'rt', encoding='utf-8') as f:
+            text = f.read()
+            xtree = astpath.file_contents_to_xml_ast(text)
+
+        # For every "import" of interest, find any surrounding "try-except-ImportError" clauses.
+        for node in xtree.xpath(f'''//Import[@lineno={lno!r}]'''):
+            tries = node.xpath('''../parent::Try[//ExceptHandler/type/Name/@id="ImportError"]''')
+            if not tries:
+                return False
+        return True