mirror of
https://github.com/mvt-project/mvt.git
synced 2026-09-03 00:21:07 +02:00
Skip modules with unavailable dependencies instead of aborting the run
A module declaring a dependency its command does not provide made _ordered_modules() give up on the whole run, so a single wrong declaration in a module scoped to several commands turned a forensic analysis into zero executed modules with one warning to explain it. Drop only the modules that cannot run: the one with the unavailable dependency, and anything depending on it. Each gets its own warning naming the module missing a dependency and the dependency it is missing, and the remaining modules run in the same stable topological order as before. A cycle in the dependency graph is still a programming error and still stops the run.
This commit is contained in:
@@ -35,9 +35,16 @@ class DependentModule(MVTModule):
|
||||
prerequisite_results = self.get_dependency_results(PrerequisiteModule)
|
||||
```
|
||||
|
||||
Selecting a single module also runs its transitive dependencies. If a dependency
|
||||
is unavailable or the dependency graph contains a cycle, the command logs a
|
||||
warning and does not run any modules.
|
||||
Selecting a single module also runs its transitive dependencies.
|
||||
|
||||
A module can only depend on modules the command it runs in also has. When a
|
||||
declared dependency is not among them, the command logs a warning naming the
|
||||
module and the missing dependency, skips that module and everything depending
|
||||
on it, and runs the rest of the analysis. Selecting such a module with
|
||||
`--module` therefore leaves nothing to run, which the warning explains.
|
||||
|
||||
A cycle in the dependency graph is a programming error rather than a
|
||||
configuration problem: the command logs a warning and runs no modules at all.
|
||||
|
||||
## Custom modules
|
||||
|
||||
|
||||
+95
-16
@@ -317,6 +317,81 @@ class Command:
|
||||
console.print("")
|
||||
console.print(panel)
|
||||
|
||||
def _skipped_modules(
|
||||
self,
|
||||
required: list[type[MVTModule]],
|
||||
module_indexes: dict[type[MVTModule], int],
|
||||
) -> dict[type[MVTModule], tuple[type[MVTModule], type[MVTModule]]]:
|
||||
"""Return the modules to drop because a dependency is unavailable.
|
||||
|
||||
A module declaring a dependency this command cannot provide is unable
|
||||
to run, and so is every module depending on it. Dropping only those
|
||||
keeps a single wrong declaration - in a module scoped to several
|
||||
commands, for example - from silencing an entire analysis.
|
||||
|
||||
The returned mapping gives, for each skipped module, the module which
|
||||
is missing a dependency and the dependency it is missing.
|
||||
"""
|
||||
skipped: dict[type[MVTModule], tuple[type[MVTModule], type[MVTModule]]] = {}
|
||||
# Skipping the one module a run was asked for leaves nothing to run,
|
||||
# which the caller reports instead.
|
||||
remainder = (
|
||||
"" if self.module_name else " The rest of the analysis will still run."
|
||||
)
|
||||
|
||||
# Skipping one module can skip the modules depending on it, which the
|
||||
# pass over the module list may already have gone past, so repeat the
|
||||
# pass until nothing changes.
|
||||
changed = True
|
||||
while changed:
|
||||
changed = False
|
||||
for module in required:
|
||||
if module in skipped:
|
||||
continue
|
||||
|
||||
for dependency in module.dependencies:
|
||||
if dependency not in module_indexes:
|
||||
skipped[module] = (module, dependency)
|
||||
changed = True
|
||||
self.log.warning(
|
||||
"Module %s will be SKIPPED: it depends on module "
|
||||
"%s, which is not available in this command.%s",
|
||||
module.__name__,
|
||||
dependency.__name__,
|
||||
remainder,
|
||||
)
|
||||
break
|
||||
|
||||
if dependency in skipped:
|
||||
root, missing = skipped[dependency]
|
||||
skipped[module] = (root, missing)
|
||||
changed = True
|
||||
if dependency is root:
|
||||
self.log.warning(
|
||||
"Module %s will be SKIPPED: it depends on "
|
||||
"module %s, itself skipped for depending on "
|
||||
"unavailable module %s.%s",
|
||||
module.__name__,
|
||||
dependency.__name__,
|
||||
missing.__name__,
|
||||
remainder,
|
||||
)
|
||||
else:
|
||||
self.log.warning(
|
||||
"Module %s will be SKIPPED: it depends on "
|
||||
"skipped module %s, in a chain starting at "
|
||||
"module %s, which depends on unavailable "
|
||||
"module %s.%s",
|
||||
module.__name__,
|
||||
dependency.__name__,
|
||||
root.__name__,
|
||||
missing.__name__,
|
||||
remainder,
|
||||
)
|
||||
break
|
||||
|
||||
return skipped
|
||||
|
||||
def _ordered_modules(self) -> Optional[list[type[MVTModule]]]:
|
||||
"""Return enabled modules in stable topological order."""
|
||||
modules = self._available_modules()
|
||||
@@ -329,30 +404,34 @@ class Command:
|
||||
else:
|
||||
selected = [module for module in modules if module.enabled]
|
||||
|
||||
required = set(selected)
|
||||
required: set[type[MVTModule]] = set()
|
||||
pending = list(selected)
|
||||
while pending:
|
||||
module = pending.pop()
|
||||
if module in required:
|
||||
continue
|
||||
required.add(module)
|
||||
for dependency in module.dependencies:
|
||||
if dependency not in module_indexes:
|
||||
self.log.warning(
|
||||
"Module %s depends on unavailable module %s. "
|
||||
"No modules will be run.",
|
||||
module.__name__,
|
||||
dependency.__name__,
|
||||
)
|
||||
return None
|
||||
if dependency not in required:
|
||||
required.add(dependency)
|
||||
# Unavailable dependencies are reported by _skipped_modules().
|
||||
if dependency in module_indexes:
|
||||
pending.append(dependency)
|
||||
|
||||
ordered_required = sorted(required, key=lambda module: module_indexes[module])
|
||||
skipped = self._skipped_modules(ordered_required, module_indexes)
|
||||
runnable = [module for module in ordered_required if module not in skipped]
|
||||
if skipped and not runnable:
|
||||
self.log.warning(
|
||||
"Every selected module was skipped for an unavailable "
|
||||
"dependency. No modules will be run."
|
||||
)
|
||||
|
||||
dependents: dict[type[MVTModule], list[type[MVTModule]]] = {
|
||||
module: [] for module in required
|
||||
module: [] for module in runnable
|
||||
}
|
||||
indegree = {module: 0 for module in required}
|
||||
for module in required:
|
||||
indegree = {module: 0 for module in runnable}
|
||||
for module in runnable:
|
||||
for dependency in module.dependencies:
|
||||
if dependency not in required:
|
||||
if dependency not in indegree:
|
||||
continue
|
||||
dependents[dependency].append(module)
|
||||
indegree[module] += 1
|
||||
@@ -371,7 +450,7 @@ class Command:
|
||||
if indegree[dependent] == 0:
|
||||
heappush(ready, (module_indexes[dependent], dependent))
|
||||
|
||||
if len(ordered) != len(required):
|
||||
if len(ordered) != len(runnable):
|
||||
cyclic_modules = sorted(
|
||||
(module.__name__ for module, count in indegree.items() if count > 0)
|
||||
)
|
||||
|
||||
@@ -157,7 +157,7 @@ class TestCommand:
|
||||
assert not hasattr(cmd, "initialized")
|
||||
assert "Circular module dependency detected" in caplog.text
|
||||
|
||||
def test_unavailable_dependency_warns_and_stops(self, caplog):
|
||||
def test_unavailable_dependency_only_skips_the_dependent_module(self, caplog):
|
||||
class UnavailableModule(RecordingModule):
|
||||
pass
|
||||
|
||||
@@ -165,14 +165,97 @@ class TestCommand:
|
||||
dependencies = (UnavailableModule,)
|
||||
|
||||
cmd = RecordingCommand()
|
||||
cmd.modules = [DependentModule]
|
||||
cmd.modules = [DependentModule, IndependentModule, FirstModule]
|
||||
|
||||
with caplog.at_level(logging.WARNING):
|
||||
cmd.run()
|
||||
|
||||
assert RecordingModule.run_order == ["IndependentModule", "FirstModule"]
|
||||
assert cmd.initialized
|
||||
assert "Module DependentModule will be SKIPPED" in caplog.text
|
||||
assert "depends on module UnavailableModule" in caplog.text
|
||||
|
||||
def test_modules_depending_on_a_skipped_module_are_skipped_too(self, caplog):
|
||||
class UnavailableModule(RecordingModule):
|
||||
pass
|
||||
|
||||
class SkippedModule(RecordingModule):
|
||||
dependencies = (UnavailableModule,)
|
||||
|
||||
class DependsOnSkippedModule(RecordingModule):
|
||||
dependencies = (SkippedModule,)
|
||||
|
||||
class DependsOnTheChain(RecordingModule):
|
||||
dependencies = (DependsOnSkippedModule,)
|
||||
|
||||
cmd = RecordingCommand()
|
||||
cmd.modules = [
|
||||
DependsOnTheChain,
|
||||
DependsOnSkippedModule,
|
||||
SkippedModule,
|
||||
IndependentModule,
|
||||
]
|
||||
|
||||
with caplog.at_level(logging.WARNING):
|
||||
cmd.run()
|
||||
|
||||
assert RecordingModule.run_order == ["IndependentModule"]
|
||||
skip_warnings = [
|
||||
record.getMessage()
|
||||
for record in caplog.records
|
||||
if "will be SKIPPED" in record.getMessage()
|
||||
]
|
||||
assert len(skip_warnings) == 3
|
||||
assert [warning.split()[1] for warning in skip_warnings] == [
|
||||
"SkippedModule",
|
||||
"DependsOnSkippedModule",
|
||||
"DependsOnTheChain",
|
||||
]
|
||||
# Every warning names the root cause: the module missing a dependency
|
||||
# and the dependency it is missing.
|
||||
assert all("UnavailableModule" in warning for warning in skip_warnings)
|
||||
assert all("module SkippedModule" in warning for warning in skip_warnings[1:])
|
||||
|
||||
def test_explicitly_selected_module_with_missing_dependency_runs_nothing(
|
||||
self, caplog
|
||||
):
|
||||
class UnavailableModule(RecordingModule):
|
||||
pass
|
||||
|
||||
class DependentModule(RecordingModule):
|
||||
dependencies = (UnavailableModule,)
|
||||
|
||||
cmd = RecordingCommand(module_name="DependentModule")
|
||||
cmd.modules = [DependentModule, IndependentModule]
|
||||
|
||||
with caplog.at_level(logging.WARNING):
|
||||
cmd.run()
|
||||
|
||||
assert RecordingModule.run_order == []
|
||||
assert not hasattr(cmd, "initialized")
|
||||
assert "depends on unavailable module UnavailableModule" in caplog.text
|
||||
assert "Module DependentModule will be SKIPPED" in caplog.text
|
||||
assert "No modules will be run" in caplog.text
|
||||
# Nothing else was selected, so the warning must not promise that the
|
||||
# analysis continues right before saying that it does not.
|
||||
assert "The rest of the analysis will still run" not in caplog.text
|
||||
|
||||
def test_unaffected_dependency_chains_keep_their_order(self, caplog):
|
||||
class UnavailableModule(RecordingModule):
|
||||
pass
|
||||
|
||||
class SkippedModule(RecordingModule):
|
||||
dependencies = (UnavailableModule,)
|
||||
|
||||
cmd = RecordingCommand()
|
||||
cmd.modules = [ThirdModule, SkippedModule, SecondModule, FirstModule]
|
||||
|
||||
with caplog.at_level(logging.WARNING):
|
||||
cmd.run()
|
||||
|
||||
assert RecordingModule.run_order == [
|
||||
"FirstModule",
|
||||
"SecondModule",
|
||||
"ThirdModule",
|
||||
]
|
||||
|
||||
def test_custom_modules_are_filtered_before_ordering(self):
|
||||
cmd = RecordingCommand()
|
||||
|
||||
Reference in New Issue
Block a user