From 2a133a562320f3af705254b33a8580eadcfc3eab Mon Sep 17 00:00:00 2001 From: Guillaume Mazoyer Date: Thu, 14 Nov 2024 08:54:10 +0100 Subject: [PATCH 1/4] Add deprecation for attributes and relationships The deprecation property takes a string as value. This value is used to convey why an attribute or relationship is deprecated. The deprecation also implies that the attribute or relationship will be marked as optional. This is enforced automatically by the schema manager. --- .../infrahub/core/schema/attribute_schema.py | 4 ++++ .../core/schema/definitions/internal.py | 16 +++++++++++++ .../core/schema/generated/attribute_schema.py | 6 +++++ .../schema/generated/relationship_schema.py | 6 +++++ .../core/schema/relationship_schema.py | 4 ++++ backend/infrahub/core/schema/schema_branch.py | 23 +++++++++++++++++++ docs/docs/reference/schema/attribute.mdx | 12 ++++++++++ docs/docs/reference/schema/relationship.mdx | 12 ++++++++++ .../reference/schema/validator-migration.mdx | 2 ++ 9 files changed, 85 insertions(+) diff --git a/backend/infrahub/core/schema/attribute_schema.py b/backend/infrahub/core/schema/attribute_schema.py index ed5183b759..00b2dcde07 100644 --- a/backend/infrahub/core/schema/attribute_schema.py +++ b/backend/infrahub/core/schema/attribute_schema.py @@ -32,6 +32,10 @@ def is_attribute(self) -> bool: def is_relationship(self) -> bool: return False + @property + def is_deprecated(self) -> bool: + return bool(self.deprecation) + @field_validator("kind") @classmethod def kind_options(cls, v: str) -> str: diff --git a/backend/infrahub/core/schema/definitions/internal.py b/backend/infrahub/core/schema/definitions/internal.py index 7f1c1b6510..32622bbfa6 100644 --- a/backend/infrahub/core/schema/definitions/internal.py +++ b/backend/infrahub/core/schema/definitions/internal.py @@ -608,6 +608,14 @@ def to_dict(self) -> dict[str, Any]: optional=True, extra={"update": UpdateSupport.ALLOWED}, ), + SchemaAttribute( + name="deprecation", + kind="Text", + optional=True, + description="Mark attribute as deprecated and provide a user-friendly message to display", + max_length=DEFAULT_DESCRIPTION_LENGTH, + extra={"update": UpdateSupport.ALLOWED}, + ), ], relationships=[ SchemaRelationship( @@ -803,6 +811,14 @@ def to_dict(self) -> dict[str, Any]: optional=True, extra={"update": UpdateSupport.ALLOWED}, ), + SchemaAttribute( + name="deprecation", + kind="Text", + optional=True, + description="Mark relationship as deprecated and provide a user-friendly message to display", + max_length=DEFAULT_DESCRIPTION_LENGTH, + extra={"update": UpdateSupport.ALLOWED}, + ), ], relationships=[ SchemaRelationship( diff --git a/backend/infrahub/core/schema/generated/attribute_schema.py b/backend/infrahub/core/schema/generated/attribute_schema.py index 26ff8c6001..b28ce0b824 100644 --- a/backend/infrahub/core/schema/generated/attribute_schema.py +++ b/backend/infrahub/core/schema/generated/attribute_schema.py @@ -115,3 +115,9 @@ class GeneratedAttributeSchema(HashableModel): description="Type of allowed override for the attribute.", json_schema_extra={"update": "allowed"}, ) + deprecation: Optional[str] = Field( + default=None, + description="Mark attribute as deprecated and provide a user-friendly message to display", + max_length=128, + json_schema_extra={"update": "allowed"}, + ) diff --git a/backend/infrahub/core/schema/generated/relationship_schema.py b/backend/infrahub/core/schema/generated/relationship_schema.py index c36e2f01ab..8efa9a2e35 100644 --- a/backend/infrahub/core/schema/generated/relationship_schema.py +++ b/backend/infrahub/core/schema/generated/relationship_schema.py @@ -125,3 +125,9 @@ class GeneratedRelationshipSchema(HashableModel): description="Set the relationship as read-only, users won't be able to change its value.", json_schema_extra={"update": "allowed"}, ) + deprecation: Optional[str] = Field( + default=None, + description="Mark relationship as deprecated and provide a user-friendly message to display", + max_length=128, + json_schema_extra={"update": "allowed"}, + ) diff --git a/backend/infrahub/core/schema/relationship_schema.py b/backend/infrahub/core/schema/relationship_schema.py index ce0fdd04d1..121f22439e 100644 --- a/backend/infrahub/core/schema/relationship_schema.py +++ b/backend/infrahub/core/schema/relationship_schema.py @@ -30,6 +30,10 @@ def is_attribute(self) -> bool: def is_relationship(self) -> bool: return True + @property + def is_deprecated(self) -> bool: + return bool(self.deprecation) + def get_class(self) -> type[Relationship]: return Relationship diff --git a/backend/infrahub/core/schema/schema_branch.py b/backend/infrahub/core/schema/schema_branch.py index 5d1f69da85..f307860b98 100644 --- a/backend/infrahub/core/schema/schema_branch.py +++ b/backend/infrahub/core/schema/schema_branch.py @@ -459,6 +459,7 @@ def process(self, validate_schema: bool = True) -> None: def process_pre_validation(self) -> None: self.generate_identifiers() self.process_default_values() + self.process_deprecations() self.process_cardinality_counts() self.process_inheritance() self.process_hierarchy() @@ -1348,6 +1349,28 @@ def process_cardinality_counts(self) -> None: self.set(name=name, schema=node) + def process_deprecations(self) -> None: + """Mark deprecated attributes and relationships as optional.""" + for name in self.all_names: + node = self.get(name=name, duplicate=False) + + change_required = False + for item in node.attributes + node.relationships: + if item.is_deprecated: + change_required = True + + if not change_required: + continue + + node = node.duplicate() + + for item in node.attributes + node.relationships: + if item.is_deprecated: + item.optional = True + log.warn(f"'{item.name}' for '{node.kind}' has been marked as deprecated, remember to clean it up") + + self.set(name=name, schema=node) + def generate_weight(self) -> None: for name in self.all_names: node = self.get(name=name, duplicate=False) diff --git a/docs/docs/reference/schema/attribute.mdx b/docs/docs/reference/schema/attribute.mdx index 071f97066b..4d33427aaf 100644 --- a/docs/docs/reference/schema/attribute.mdx +++ b/docs/docs/reference/schema/attribute.mdx @@ -20,6 +20,7 @@ Below is the list of all available options to define an Attribute in the schema | [**choices**](#choices) | Attribute | Define a list of valid choices for a dropdown attribute. | False | | [**computed_attribute**](#computed_attribute) | Attribute | Defines how the value of this attribute will be populated. | False | | [**default_value**](#default_value) | Attribute | Default value of the attribute. | False | +| [**deprecation**](#deprecation) | Attribute | Mark attribute as deprecated and provide a user-friendly message to display | False | | [**description**](#description) | Attribute | Short description of the attribute. | False | | [**enum**](#enum) | Attribute | Define a list of valid values for the attribute. | False | | [**kind**](#kind) | Attribute | Defines the type of the attribute. | True | @@ -114,6 +115,17 @@ extensions: | **Default Value** | | | **Constraints** | | +### deprecation + +| Key | Value | +| ---- | --------------- | +| **Name** | deprecation | +| **Kind** | `Text` | +| **Description** | Mark attribute as deprecated and provide a user-friendly message to display | +| **Optional** | True | +| **Default Value** | | +| **Constraints** | Length: min -, max 128 | + ### description | Key | Value | diff --git a/docs/docs/reference/schema/relationship.mdx b/docs/docs/reference/schema/relationship.mdx index 43b48312b4..06542fa5f0 100644 --- a/docs/docs/reference/schema/relationship.mdx +++ b/docs/docs/reference/schema/relationship.mdx @@ -18,6 +18,7 @@ Below is the list of all available options to define a Relationship in the schem | [**allow_override**](#allow_override) | Attribute | Type of allowed override for the relationship. | False | | [**branch**](#branch) | Attribute | Type of branch support for the relatioinship, if not defined it will be determine based both peers. | False | | [**cardinality**](#cardinality) | Attribute | Defines how many objects are expected on the other side of the relationship. | False | +| [**deprecation**](#deprecation) | Attribute | Mark relationship as deprecated and provide a user-friendly message to display | False | | [**description**](#description) | Attribute | Short description of the relationship. | False | | [**direction**](#direction) | Attribute | Defines the direction of the relationship, Unidirectional relationship are required when the same model is on both side. | False | | [**hierarchical**](#hierarchical) | Attribute | Internal attribute to track the type of hierarchy this relationship is part of, must match a valid Generic Kind | False | @@ -72,6 +73,17 @@ Below is the list of all available options to define a Relationship in the schem | **Constraints** | | | **Accepted Values** | `one` `many` | +### deprecation + +| Key | Value | +| ---- | --------------- | +| **Name** | deprecation | +| **Kind** | `Text` | +| **Description** | Mark relationship as deprecated and provide a user-friendly message to display | +| **Optional** | True | +| **Default Value** | | +| **Constraints** | Length: min -, max 128 | + ### description | Key | Value | diff --git a/docs/docs/reference/schema/validator-migration.mdx b/docs/docs/reference/schema/validator-migration.mdx index 0aa2f46502..94d42f8092 100644 --- a/docs/docs/reference/schema/validator-migration.mdx +++ b/docs/docs/reference/schema/validator-migration.mdx @@ -68,6 +68,7 @@ In this context, an element represent either a Node, a Generic, an Attribute or | **order_weight** | allowed | | **default_value** | allowed | | **allow_override** | allowed | +| **deprecation** | allowed | ### Relationship @@ -91,6 +92,7 @@ In this context, an element represent either a Node, a Generic, an Attribute or | **on_delete** | allowed | | **allow_override** | allowed | | **read_only** | allowed | +| **deprecation** | allowed | ### Generic From c3ed38b6275b1215cf1c434facab4791c3e2b166 Mon Sep 17 00:00:00 2001 From: Guillaume Mazoyer Date: Thu, 14 Nov 2024 09:42:55 +0100 Subject: [PATCH 2/4] Validate deprecation processing --- .../schema_manager/test_manager_schema.py | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/backend/tests/unit/core/schema_manager/test_manager_schema.py b/backend/tests/unit/core/schema_manager/test_manager_schema.py index 1b1f4bce9c..695712b790 100644 --- a/backend/tests/unit/core/schema_manager/test_manager_schema.py +++ b/backend/tests/unit/core/schema_manager/test_manager_schema.py @@ -2571,3 +2571,48 @@ def test_schema_branch_conflicting_required_relationships(schema_all_in_one): assert "BuiltinTag" in exc.value.args[0] assert "BuiltinCriticality" in exc.value.args[0] assert "cannot both have required relationships" in exc.value.args[0] + + +async def test_process_deprecations(organization_schema): + SCHEMA1 = { + "name": "Criticality", + "namespace": "Test", + "default_filter": "name__value", + "branch": BranchSupportType.AWARE.value, + "attributes": [ + {"name": "name", "kind": "Text", "unique": True}, + {"name": "description", "kind": "Text", "deprecation": "I'm not used anymore"}, + ], + "relationships": [ + { + "name": "first", + "peer": "CoreOrganization", + "cardinality": "one", + "optional": False, + "deprecation": "Use the second one instead", + }, + {"name": "second", "peer": "CoreOrganization", "cardinality": "one", "optional": False}, + ], + } + + copy_core_models = copy.deepcopy(core_models) + copy_core_models["nodes"].append(SCHEMA1) + schema = SchemaRoot(**copy_core_models) + + schema_branch = SchemaBranch(cache={}, name="test") + schema_branch.load_schema(schema=schema) + schema_branch.load_schema(schema=organization_schema) + + schema_branch.process_deprecations() + + test_criticality = schema_branch.get_node(name="TestCriticality", duplicate=False) + + assert not test_criticality.get_attribute(name="name").is_deprecated + assert test_criticality.get_attribute(name="description").is_deprecated + assert test_criticality.get_relationship(name="first").is_deprecated + assert not test_criticality.get_relationship(name="second").is_deprecated + + assert not test_criticality.get_attribute(name="name").optional + assert test_criticality.get_attribute(name="description").optional + assert test_criticality.get_relationship(name="first").optional + assert not test_criticality.get_relationship(name="second").optional From bf563c58b3074e3561391060fa41e91ae6197318 Mon Sep 17 00:00:00 2001 From: Guillaume Mazoyer Date: Fri, 15 Nov 2024 12:06:58 +0100 Subject: [PATCH 3/4] Add changelog entry --- changelog/4245.added.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/4245.added.md diff --git a/changelog/4245.added.md b/changelog/4245.added.md new file mode 100644 index 0000000000..8702e67fed --- /dev/null +++ b/changelog/4245.added.md @@ -0,0 +1 @@ +Add a "deprecation" property to attribute and relationship schema in order to allow users to identify deprecated fields for nodes and provide a user-friendly message about the deprecation reasons. \ No newline at end of file From c9ae81898208b261a552e398e6fed2943f3df307 Mon Sep 17 00:00:00 2001 From: Guillaume Mazoyer Date: Wed, 20 Nov 2024 18:06:28 +0100 Subject: [PATCH 4/4] Only change optional value if needed --- backend/infrahub/core/schema/schema_branch.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/backend/infrahub/core/schema/schema_branch.py b/backend/infrahub/core/schema/schema_branch.py index f307860b98..d78e186995 100644 --- a/backend/infrahub/core/schema/schema_branch.py +++ b/backend/infrahub/core/schema/schema_branch.py @@ -1357,7 +1357,9 @@ def process_deprecations(self) -> None: change_required = False for item in node.attributes + node.relationships: if item.is_deprecated: - change_required = True + log.warn(f"'{item.name}' for '{node.kind}' has been marked as deprecated, remember to clean it up") + if not item.optional: + change_required = True if not change_required: continue @@ -1365,9 +1367,8 @@ def process_deprecations(self) -> None: node = node.duplicate() for item in node.attributes + node.relationships: - if item.is_deprecated: + if item.is_deprecated and not item.optional: item.optional = True - log.warn(f"'{item.name}' for '{node.kind}' has been marked as deprecated, remember to clean it up") self.set(name=name, schema=node)