Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CT-2522] Redefine Metric node for the Manifest and update parsing #7500

Closed
Tracked by #7498
QMalcolm opened this issue May 3, 2023 · 2 comments · Fixed by #7812
Closed
Tracked by #7498

[CT-2522] Redefine Metric node for the Manifest and update parsing #7500

QMalcolm opened this issue May 3, 2023 · 2 comments · Fixed by #7812
Assignees

Comments

@QMalcolm
Copy link
Contributor

QMalcolm commented May 3, 2023

There are two new node types being added to the manifest as part of the MetricFlow integration work: SemanticModel nodes and Metric nodes. This ticket addresses the Metric nodes. In dbt-semantic-interfaces we're adding a protocol of a Metric, which will have the shared standard definition of a Metric. In dbt-core we need to compose this protocol with base node classes to define dbt-core's representation of a Metric. Additionally there are some additional fields we should expose as defined in #7456

Blocked by:

@emmyoop
Copy link
Member

emmyoop commented May 22, 2023

This should be done along with the issue to parse the new metric node so that we don't start breaking tests - #7501

@dbeatty10 dbeatty10 removed the triage label May 22, 2023
@QMalcolm QMalcolm changed the title [CT-2522] Define Metric node for the Manifest [CT-2522] Redefine Metric node for the Manifest and update parsing May 30, 2023
@QMalcolm
Copy link
Contributor Author

An update on context. dbt-core already has a concept of a Metric Node from dbt-metrics. Thus instead removing and re-adding metric nodes, what we're really doing is redefining metric nodes to match the spec defined in dbt-semantic-interfaces. Additionally, since metric nodes already exist we don't need to build parsing for them, but instead just update the parsing. Thus #7501 is being wrapped into the work of this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants