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-2752] Run DSI SemanticManifest validations during parsing #7969

Closed
Tracked by #7498
QMalcolm opened this issue Jun 27, 2023 · 0 comments · Fixed by #8049
Closed
Tracked by #7498

[CT-2752] Run DSI SemanticManifest validations during parsing #7969

QMalcolm opened this issue Jun 27, 2023 · 0 comments · Fixed by #8049
Assignees
Labels
enhancement New feature or request semantic Issues related to the semantic layer

Comments

@QMalcolm
Copy link
Contributor

QMalcolm commented Jun 27, 2023

In dbt-semantic-interfaces there lives a tool called SemanticManifestValidator. The SemanticManifestValidator takes in a semantic manifest and runs a series of rules against it to ensure that it is valid. These are things like ensuring that either a measure has an agg_time_dimension set or that there is a default_agg_time_dimension set on the semantic_model that the measure belongs to.

Currently we don't run these validation rules in core. The only way someone using MetricFlow with core can run the validations is by doing

$ dbt parse
...
$ mf validate-configs

The issue with this is that validations are only run after the semantic manifest is written. This is problematic because validation issues should cause a failure before writing the semantic manifest. Additionally most people likely won't run mf validate-configs separately. Which means the issues would only be caught at query time 😬 We want to be informing customers of their issues when writing their configs, thus we need to ensure the SemanticManifestValidator is run during parsing.

Luckily, using the SemanticManifestValidator is fairly easy. It's a generic python class that requires that you declare an implementation of the SemanticManifest protocol on instantiation. It would look something like

validator = SemanticManifestValidator[SemanticManifestImplementation]()
validation_results = validator.validate(semantic_manifest)

The other part of this is presently we only construct the SemanticManifest, and specifically a PydanticSemanticManifest right before we write out the semantic manifest. We can either reuse the PydanticSemanticManifest builder to get a PydanticSemanticManifest and run that through a SemanticManifestValidator or we'll need to actually create a core implementation of the SemanticManifest protocol and then run that through a SemanticManifestValidator. The former is easier, but the latter is probably more in core's style.

Acceptance Criteria

  1. If a manifest has semantic models and/or metrics, the SemanticManifestValidator should get run as part of parsing
    • It needs happen after nodes have been fully "processed" (i.e. after things like this)
    • It needs to happen before writing out the semantic manifest
  2. Validation issues returned by the validation process should get logged
  3. If there are ERROR level validation issues, parsing should be considered as having failed.
@QMalcolm QMalcolm added the semantic Issues related to the semantic layer label Jun 27, 2023
@QMalcolm QMalcolm added this to the v1.6 milestone Jun 27, 2023
@github-actions github-actions bot changed the title Run DSI SemanticManifest validations during pparsing [CT-2752] Run DSI SemanticManifest validations during pparsing Jun 27, 2023
@QMalcolm QMalcolm added the enhancement New feature or request label Jun 27, 2023
@QMalcolm QMalcolm changed the title [CT-2752] Run DSI SemanticManifest validations during pparsing [CT-2752] Run DSI SemanticManifest validations during parsing Jun 28, 2023
@jtcohen6 jtcohen6 removed this from the v1.6 milestone Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request semantic Issues related to the semantic layer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants