From 899d23b77fef919fc9c43de96a7ada57e0df85d3 Mon Sep 17 00:00:00 2001 From: Adam Hillier <7688302+AdamHillier@users.noreply.github.com> Date: Wed, 18 Mar 2020 21:35:13 +0000 Subject: [PATCH] Disallow configuring non-component instances. Fixes #130. (#131) --- zookeeper/core/component.py | 7 +++++++ zookeeper/core/component_test.py | 34 ++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/zookeeper/core/component.py b/zookeeper/core/component.py index 7944fa6..d5f0f7a 100644 --- a/zookeeper/core/component.py +++ b/zookeeper/core/component.py @@ -474,6 +474,13 @@ def configure( overwrite any values already set on the instance - either class defaults or those set in `__init__`. """ + # Only component instances can be configured. + if not utils.is_component_instance(instance): + raise TypeError( + "Only @component, @factory, and @task instances can be configured. " + f"Received: {instance}." + ) + # Configuration can only happen once. if instance.__component_configured__: raise ValueError( diff --git a/zookeeper/core/component_test.py b/zookeeper/core/component_test.py index 0d188ba..6c4764a 100644 --- a/zookeeper/core/component_test.py +++ b/zookeeper/core/component_test.py @@ -523,6 +523,40 @@ class GrandParent: configure(GrandParent(), {"parent.non_existent_field": "bar"}) +def test_component_configure_error_non_component_instance(): + class A: + a: int = Field() + + with pytest.raises( + TypeError, + match="Only @component, @factory, and @task instances can be configured.", + ): + configure(A(), conf={"a": 5}) + + @component + class B: + b: int = Field() + + with pytest.raises( + TypeError, + match="Only @component, @factory, and @task instances can be configured.", + ): + # The following we expect to fail because it is a component class, not + # an instance. + configure(B, conf={"b": 3}) + + class C(B): + c: int = Field() + + with pytest.raises( + TypeError, + match="Only @component, @factory, and @task instances can be configured.", + ): + # Even the an instance of a class that subclasses a component class + # should fail. + configure(C(), conf={"b": 3, "c": 42}) + + def test_component_configure_field_allow_missing(): @component class A: