Rename Identifier -> Entity #490

WilliamDee · 2023-05-02T21:57:18Z

resolves dbt-labs/dbt-semantic-interfaces#9

Description

In the new world of dbt-core x MetricFlow Identifiers are becoming Entities. Additionally some of the properties of the object are changing. The resulting object should have the following properties

Property Name	Type	Description
name	str	Name of the entity
type	enum	Type of the entity
description	str	Description of the entity
role	str	Role of the entity
entities	List[str]	List of composite sub-entities
expr	str	Expression of the entity

The above properties are were pulled from dbt-labs/dbt-core#7456

github-actions · 2023-05-02T21:57:35Z

Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide.

tlento

Working through it, need to make dinner. Renaming things is so terrible, sorry.

tlento · 2023-05-04T00:50:15Z

dbt_semantic_interfaces/dbt_semantic_interfaces/objects/elements/entity.py

+class Entity(HashableBaseModel, ModelWithMetadataParsing):
+    """Describes a entity"""
+
+    name: str
+    description: Optional[str]
+    type: EntityType
+    role: Optional[str]
+    entities: List[CompositeSubEntity] = []
+    expr: Optional[str] = None
+


I think we should leave the metadata key here for now, as it's how we track lineage internally.

@QMalcolm this might be the time to rename metadata to something more descriptive, like lineage_metadata or lineage_info or something.

Update: We've decided to keep it for now. We might not use it right away, and we might refine it slightly before we do. The metadata as a concept can be incredibly useful for user experience.

tlento · 2023-05-04T00:50:58Z

dbt_semantic_interfaces/dbt_semantic_interfaces/objects/elements/entity.py

+    UNIQUE = "unique"
+
+
+class CompositeSubEntity(HashableBaseModel):


This makes everything so gnarly, but let's just move it over for now and decide what to do with it later.

tlento · 2023-05-04T01:02:03Z

metricflow/dataset/convert_data_source.py

@@ -294,16 +294,16 @@ def _convert_dimensions(
            select_columns=select_columns,
        )

-    def _create_identifier_instances(
+    def _create_entity_instances(


I am up to here.

WilliamDee · 2023-05-04T19:28:30Z

dbt_semantic_interfaces/dbt_semantic_interfaces/objects/elements/entity.py

+    role: Optional[str]
+    entities: List[CompositeSubEntity] = []
+    expr: Optional[str] = None
+    metadata: Optional[Metadata] = None


We had it as Optional[Metadata] without a default value. I added the default of None to allow not having to explicitly set it to None if we don't pass this property in.

WilliamDee · 2023-05-04T19:38:09Z

dbt_semantic_interfaces/pyproject.toml

@@ -24,3 +24,23 @@ more-itertools = "8.10.0"
 [build-system]
 requires = ["poetry-core>=1.0.0"]
 build-backend = "poetry.core.masonry.api"
+
+[tool.black]


@QMalcolm added this cause there was some CI issues with different black configurations between MF and this repo.

QMalcolm

Review of first 10 commits. Will review the rest after lunch.

dbt_semantic_interfaces/dbt_semantic_interfaces/objects/elements/entity.py

metricflow/test/model/parsing/test_data_source_parsing.py

tlento

Whew. Massive!

Test assertion change is odd, what's up there?

tlento · 2023-05-04T19:59:17Z

metricflow/test/model/test_data_source_container.py

@@ -197,4 +197,4 @@ def test_local_linked_elements_for_metric(metric_semantics: MetricSemantics) ->
 def test_get_data_sources_for_entity(data_source_semantics: DataSourceSemantics) -> None:  # noqa: D
    entity_reference = EntityReference(element_name="user")
    linked_data_sources = data_source_semantics.get_data_sources_for_entity(entity_reference=entity_reference)
-    assert len(linked_data_sources) == 9
+    assert len(linked_data_sources) == 8


Wait what? Why?

So basically, when I removed the entity property on the Identifier class, I removed all the entity fields on the test models. One of them was this 200445c#diff-e939351cfbeb60d978e42432cbfb25ebba45ff60f4ca48457712dad0eefd0ce3 and from get_data_sources_for_entity("user"), that bookings_source data source is no longer returned from that function call.

Ok cool, that makes sense, thanks!

metricflow/test/model/validations/test_entities.py

QMalcolm

Finished my first pass! 😂

QMalcolm · 2023-05-04T20:26:20Z

metricflow/dataflow/builder/node_evaluator.py

-                    raise RuntimeError(
-                        f"Could not find identifier instance with name ({identifier_spec_in_right_node})"
-                    )
+                    raise RuntimeError(f"Could not find identifier instance with name ({entity_spec_in_right_node})")


f"Could not find identifier ... -> f"Could not find entity ...

Looks like you caught this already, nice!

tlento

LGTM, assuming all of the various snapshot tests pass across engines and @QMalcolm doesn't turn up any issues.

QMalcolm

I think we should dub this "The Great Renamen-ing". This looks good to me! (contingent that the remaining references to identifers / identifer are planned to go away in the renaming of DataSource to SemanticModel

cla-bot bot added the cla:yes label May 2, 2023

WilliamDee force-pushed the will/rename-identifiers branch 9 times, most recently from 7751fd3 to ea39e77 Compare May 3, 2023 18:37

WilliamDee marked this pull request as ready for review May 3, 2023 18:40

WilliamDee force-pushed the will/rename-identifiers branch 2 times, most recently from b885b6f to 442aae0 Compare May 3, 2023 18:59

WilliamDee requested review from plypaul, tlento and QMalcolm May 3, 2023 19:02

tlento added the run_mf_sql_engine_tests label May 3, 2023

tlento temporarily deployed to DW_INTEGRATION_TESTS May 3, 2023 22:01 — with GitHub Actions Inactive

tlento reviewed May 4, 2023

View reviewed changes

WilliamDee requested a review from tlento May 4, 2023 18:04

WilliamDee force-pushed the will/rename-identifiers branch from 442aae0 to 0bf7676 Compare May 4, 2023 19:26

WilliamDee commented May 4, 2023

View reviewed changes

QMalcolm reviewed May 4, 2023

View reviewed changes

dbt_semantic_interfaces/dbt_semantic_interfaces/objects/elements/entity.py Outdated Show resolved Hide resolved

metricflow/test/model/parsing/test_data_source_parsing.py Outdated Show resolved Hide resolved

tlento reviewed May 4, 2023

View reviewed changes

WilliamDee added 17 commits May 4, 2023 16:08

rename LinkableIdentifier -> LinkableEntity

decc5be

updated *Identifier* classes to *Entity*

9f65b2a

change all Identifier.identifiers -> Entity.entities

9b379e2

renamed identifier_link -> entity_link

de14d28

fix column resolver

f393bb7

rename identifier_spec -> entity_spec

bcc3199

rename identifier_instance -> entity_instance

fed80e0

rename identifier_column -> entity_column

68bb432

rename identifier_reference -> entity_reference

1c9385c

renamed _identifier -> _entity

551626d

remove entity param from tests

27be13c

add black linting configuration to dbt_semantic_interfaces

6391b95

rename comments and missed enum classes

a947094

fix tests

e4d1e8e

remove renamed snapshots

08cc37f

update snapshots

edc46a7

added changelog

d6becba

WilliamDee force-pushed the will/rename-identifiers branch from 3830f87 to d6becba Compare May 4, 2023 20:27

QMalcolm reviewed May 4, 2023

View reviewed changes

tlento added run_mf_sql_engine_tests and removed run_mf_sql_engine_tests labels May 4, 2023

tlento temporarily deployed to DW_INTEGRATION_TESTS May 4, 2023 21:20 — with GitHub Actions Inactive

tlento approved these changes May 4, 2023

View reviewed changes

QMalcolm approved these changes May 4, 2023

View reviewed changes

WilliamDee merged commit 97a7ed3 into main May 4, 2023

WilliamDee deleted the will/rename-identifiers branch May 4, 2023 22:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename Identifier -> Entity #490

Rename Identifier -> Entity #490

WilliamDee commented May 2, 2023

github-actions bot commented May 2, 2023

tlento left a comment

tlento May 4, 2023

QMalcolm May 4, 2023

tlento May 4, 2023

tlento May 4, 2023

WilliamDee May 4, 2023

WilliamDee May 4, 2023

QMalcolm left a comment

tlento left a comment

tlento May 4, 2023

WilliamDee May 4, 2023

tlento May 4, 2023

QMalcolm left a comment

QMalcolm May 4, 2023 •

edited

Loading

QMalcolm May 4, 2023

tlento left a comment

QMalcolm left a comment

		UNIQUE = "unique"


		class CompositeSubEntity(HashableBaseModel):

Rename Identifier -> Entity #490

Rename Identifier -> Entity #490

Conversation

WilliamDee commented May 2, 2023

Description

github-actions bot commented May 2, 2023

tlento left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

QMalcolm left a comment

Choose a reason for hiding this comment

tlento left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

QMalcolm left a comment

Choose a reason for hiding this comment

QMalcolm May 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tlento left a comment

Choose a reason for hiding this comment

QMalcolm left a comment

Choose a reason for hiding this comment

QMalcolm May 4, 2023 •

edited

Loading