-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Propagate the measure label when using create_metric option #10536
Propagate the measure label when using create_metric option #10536
Conversation
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. |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #10536 +/- ##
=======================================
Coverage 88.84% 88.84%
=======================================
Files 180 180
Lines 22717 22720 +3
=======================================
+ Hits 20182 20186 +4
+ Misses 2535 2534 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
9342b0a
to
be26928
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. It would probably be worthwhile to add a test which checks the state of the created metric when the related measure does not have a label set, however this is not blocking.
Context
When _create_metric
, which creates a metric from a measure, was written, measures did not have labels. It seems that when label
was added to the measure class, we did not think to add it here. That is an oversight, but I can see how that happened.
Implications
For metrics created from measures, in order to get the correct label, a full reparse will be necessary. This is because partial parsing only reparses an object is the input for the object has changed. Thus, without a full parsing, the relevant metrics will only get updated if the underlying semantic model is modified somehow.
TL;DR
Looks good. People will need to run dbt parse --no-partial-parse
to ensure all existing measures with labels that create metrics properly propagate the label.
be26928
to
402c4c6
Compare
402c4c6
to
e867392
Compare
@QMalcolm thanks for the context! I updated the test. I don't have permission to merge so I'll leave the merging to you |
Resolves #10538
Problem
When using create_metric on a measure, the label of the measure is not propagated to the metric
Solution
Set metric label to be measure label if it exists
Checklist