Merge pull request #487 from flyingcircusio/fix-dependency-error

Fix unused component detection

.gitignore

@@ -33,3 +33,4 @@ pip-wheel-metadata
 /examples/**/insecure-private.key
 *.status
 *.orig
+/build/

CHANGES.d/20241118_114301_cz_regression.md

@@ -0,0 +1,3 @@
+- Fix unused component detection
+
+  There was a bug in unused component detection which was triggered occasionally. It is not entirely clear what triggered it, but it "helped" to have a lot of components.

requirements-dev.txt

@@ -1,4 +1,4 @@
--e .
+-e .[test]
 appdirs==1.4.4
 cfgv==3.2.0
 distlib==0.3.8

src/batou/__init__.py

@@ -567,9 +567,9 @@ class UnsatisfiedResources(ConfigurationError):
     def from_context(cls, resources):
         self = cls()
         self.unsatisfied_resources = []
-        for key in sorted(resources.keys()):
+        for (key, host), res_for_key in sorted(resources.items()):
             self.unsatisfied_resources.append(
-                (key, [r.name for r in resources[key]])
+                (key, host, [r.name for r in res_for_key])
             )
         return self
 
@@ -578,13 +578,20 @@ def __str__(self):
 
     def report(self):
         output.error("Unsatisfied resource requirements")
-        for key, resources in self.unsatisfied_resources:
-            output.line(
-                '    Resource "{}" required by {}'.format(
-                    key, ",".join(resources)
-                ),
-                red=True,
-            )
+        for key, host, resources in self.unsatisfied_resources:
+            if host is None:
+                msg = (
+                    f'    Resource "{key}" required by '
+                    f'{",".join(resources)}'
+                )
+
+            else:
+                msg = (
+                    f'    Resource "{key}" on "{host}" required by '
+                    f'{",".join(resources)}'
+                )
+
+            output.line(msg, red=True)
 
 
 class MissingEnvironment(ConfigurationError):

src/batou/resources.py

@@ -153,7 +153,7 @@ def unsatisfied(self):
             if not any(s.strict for s in subscribers):
                 continue
             if key not in self.resources:
-                unsatisfied.add(key)
+                unsatisfied.add((key, None))
                 continue
             for s in subscribers:
                 if s.host is None:
@@ -162,25 +162,25 @@ def unsatisfied(self):
                     resource_root.host is s.host
                     for resource_root in self.resources[key]
                 ):
-                    unsatisfied.add(key)
+                    unsatisfied.add((key, s.host.name))
                     break
         return unsatisfied
 
     @property
     def unsatisfied_components(self):
         components = set()
-        for resource in self.unsatisfied:
+        for resource, host in self.unsatisfied:
             components.update(
-                [s.root for s in self._subscriptions(resource, None)]
+                [s.root for s in self._subscriptions(resource, host)]
             )
         return components
 
     @property
     def unsatisfied_keys_and_components(self):
         keys = {}
-        for resource in self.unsatisfied:
-            keys[resource] = set(
-                [s.root for s in self._subscriptions(resource, None)]
+        for resource, host in self.unsatisfied:
+            keys[(resource, host)] = set(
+                [s.root for s in self._subscriptions(resource, host)]
             )
         return keys
 

src/batou/tests/test_config.py

@@ -122,7 +122,7 @@ def test_config_exceptions_orderable(env):
         ),
         batou.UnknownComponentConfigurationError.from_context(c.root, ex, tb),
         batou.UnusedResources.from_context({"asdf": {c.root: 1}}),
-        batou.UnsatisfiedResources.from_context({"asdf": [c.root]}),
+        batou.UnsatisfiedResources.from_context({("asdf", None): [c.root]}),
         batou.MissingEnvironment.from_context(env),
         batou.MissingComponent.from_context("asdf", "localhost"),
         batou.SuperfluousSection.from_context("asdf"),

src/batou/tests/test_dependencies.py

@@ -84,12 +84,12 @@ def test_consumer_retrieves_value_from_provider_order1(env):
 
 def test_provider_with_consumer_limited_by_host_raises_error(env):
     env.add_root("provider", Host("test2", env))
+    env.add_root("consumer", Host("test2", env))
     env.add_root("samehostconsumer", Host("test", env))
     errors = env.configure()
-    assert len(errors) == 3
+    assert len(errors) == 2
     assert isinstance(errors[0], UnsatisfiedResources)
     assert isinstance(errors[1], NonConvergingWorkingSet)
-    assert isinstance(errors[2], UnusedResources)
 
 
 def test_consumer_retrieves_value_from_provider_order2(env):
@@ -106,7 +106,7 @@ def test_consumer_without_provider_raises_error(env):
     for exc in env.exceptions:
         if isinstance(exc, UnsatisfiedResources):
             assert set(["the-answer"]) == set(
-                [key for key, _ in exc.unsatisfied_resources]
+                [key for key, _, _ in exc.unsatisfied_resources]
             )
             break
     else:
@@ -119,7 +119,7 @@ def test_aggressive_consumer_raises_unsatisfiedrequirement(env):
     for exc in env.exceptions:
         if isinstance(exc, UnsatisfiedResources):
             assert set(["the-answer"]) == set(
-                [key for key, _ in exc.unsatisfied_resources]
+                [key for key, _, _ in exc.unsatisfied_resources]
             )
             break
     else:

src/batou/tests/test_exceptions.py

@@ -56,7 +56,7 @@ def test_configurationerrors_can_be_sorted(root):
 
     errors.append(UnusedResources.from_context({"asdf": {root: 1}}))
 
-    errors.append(UnsatisfiedResources.from_context({"asdf": [root]}))
+    errors.append(UnsatisfiedResources.from_context({("asdf", None): [root]}))
 
     errors.append(MissingEnvironment.from_context(root.environment))
 

src/batou/tests/test_resources.py

@@ -34,4 +34,7 @@ def test_mentions_missing_requirement_with_host_requirement(sample_service):
     assert len(errors) == 2
     assert isinstance(errors[0], UnsatisfiedResources)
     assert isinstance(errors[1], NonConvergingWorkingSet)
-    assert "key" in str(errors[0].__dict__)
+    assert errors[0].unsatisfied_resources == [
+        ("key", "host1", []),
+        ("unrelated", None, ["component1"]),
+    ]