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-3042] Support meta attribute for SemanticModel nodes #8511

Closed
Tracked by #8125
QMalcolm opened this issue Aug 29, 2023 · 5 comments · Fixed by #8754
Closed
Tracked by #8125

[CT-3042] Support meta attribute for SemanticModel nodes #8511

QMalcolm opened this issue Aug 29, 2023 · 5 comments · Fixed by #8754
Assignees
Labels
enhancement New feature or request Impact: CA semantic Issues related to the semantic layer

Comments

@QMalcolm
Copy link
Contributor

QMalcolm commented Aug 29, 2023

It was recently noted by a user in a issue created on the MetricFlow project that the meta attribute isn't supported for SemanticModel nodes. As meta is a manifest node attribute (and not needed currently in MetricFlow or MetricFlowServer), I've moved the request here.

Acceptance Criteria

  • A meta field which is an arbitrary dictionary can be specified for SemanticModels and makes it to the corresponding SemanticModel node in the manifest
  • A test exists for ensuring this functionality
@github-actions github-actions bot changed the title Support meta attribute for SemanticModel nodes [CT-3042] Support meta attribute for SemanticModel nodes Aug 29, 2023
@QMalcolm QMalcolm added enhancement New feature or request semantic Issues related to the semantic layer labels Aug 29, 2023
@QMalcolm
Copy link
Contributor Author

Is meta an attribute we require for all manifest nodes? If so perhaps we should consider creating a ManifestNode protocol that defines the fields which all nodes must support, and then use that protocol for some type checking.

@QMalcolm
Copy link
Contributor Author

It's worth noting that the original issue denoted a desire to have meta propagated to the semantic_manifest.json. This is not currently possible, this is because

  1. dbt-semantic-interfaces de-serialization does not allow additional properties
  2. dbt-semantic-interfaces protocols only contain attributes needed by both dbt-core and MetricFlow
  3. the meta attribute is not needed in MetricFlow

Thus because of (3), meta is not part of the DSI protocol spec (2). Which in turn means we can't put it in the semantic_manifest.json because (1) means de-serialization would break. It is possible that long term we can change the de-serialization process in DSI to allow additional properties to exist in the serialization without causing de-serialization exceptions, but that is a separate conversation.

@jtcohen6
Copy link
Contributor

@QMalcolm There's a little bit of history here:

  • Originally, meta was a top-level attribute of (most/all) manifest nodes
  • We then allowed users to start defining it as a "config": in config: blocks in yaml files, and for all nodes in a subdirectory of the resource type in dbt_project.yml
  • For backwards compatibility, we continued to include meta both as a top-level node attribute and nested within the config dictionary, within the serialized node representation (in manifest.json)

I would expect users to define meta as a nested attribute within the config dict for semantic models. (We're adding support for this in #8502.)

semantic_models:
  - name: my_sem_mod
    config:
      meta:
        whatever_key: whatever_value

If we can't then, during serialization, copy meta to be a top-level attribute of semantic_models, as we do with other node types — that would be a bit inconsistent for metadata consumers, but it's definitely not a show-stopper.

@jtcohen6
Copy link
Contributor

jtcohen6 commented Sep 8, 2023

In conversation with @marcodamore @tlento @QMalcolm last week, we weren't sure whether:

  1. meta can be a "bonus" attribute within dbt-core only, serialized out to manifest.json (current scope of this issue)
  2. meta is needed for functionality within MetricFlow / needs to be included in semantic_manifest.json, in which case we would also need to add it to the DSI protocols

Update: We have decided on (1) for the time being. config and meta are for dbt-core only.

@ataft
Copy link

ataft commented Sep 25, 2023

I'm not very familiar with how dbt-semantic-interfaces and dbt-core interact, so am not completely following. At the end of the day, the goal is to have meta available as an attribute in semantic interfaces. Rather than defining meta in the model/column YAML, I would define it in the semantic interfaces meta attribute on the entity/metric/dimension. Like many others, I am using meta for semantic layer purposes (synonyms, formatting, labels, etc.). With the new semantic layer and separate YAML definitions, these attributes should be in one place rather than defined partially in model YAML and partially in semantic YAML. Thus, the need for meta on semantic layer entity/metric/dimension. I do not want/need meta from dbt-core model YAML to be available in the semantic manifest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Impact: CA semantic Issues related to the semantic layer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants