fix (dbt): derived metrics #154
Conversation
betodealmeida
force-pushed
the
issue_153
branch
from
395c233
to
9f706a1
Compare
| @@ -150,7 +150,7 @@ def dbt_core( # pylint: disable=too-many-arguments, too-many-locals | |||
| for config in configs["metrics"].values(): | |||
| # conform to the same schema that dbt Cloud uses for metrics | |||
| config["dependsOn"] = config["depends_on"]["nodes"] | |||
| config["uniqueID"] = config["unique_id"] | |||
| config["uniqueId"] = config["unique_id"] | |||
There was a problem hiding this comment.
This was a bug, but since the metric unique ID was not needed anywhere it was not caught. With the changes in this PR we use the metric ID to traverse the DAG upstream to find parent models.
| metric["name"]: metric | ||
| for metric in metrics | ||
| if model["unique_id"] in metric["depends_on"] |
There was a problem hiding this comment.
This was too naive, and didn't account for how derived metrics depend on other metrics, instead of depending on models. I replaced it with a smarter function get_metrics_for_model that traverses the DAG upstream.
| if is_derived(node): | ||
| queue.extend(metric_map[parent] for parent in depends_on) |
There was a problem hiding this comment.
This is the crux of the PR: for derived metrics we need to look at the upstream metrics to find the parent models.
There was a problem hiding this comment.
is the metric list something from superset or something from the dbt manifest?
| if "calculation_method" in metric: | ||
| # dbt >= 1.3 | ||
| type_ = metric["calculation_method"] | ||
| sql = metric["expression"] | ||
| expression = "derived" | ||
| else: | ||
| # dbt < 1.3 | ||
| type_ = metric["type"] | ||
| sql = metric["sql"] | ||
| expression = "expression" |
| @@ -22,8 +25,16 @@ def get_metric_expression(metric_name: str, metrics: Dict[str, MetricSchema]) -> | |||
| raise Exception(f"Invalid metric {metric_name}") | |||
|
|
|||
| metric = metrics[metric_name] | |||
| type_ = metric["type"] | |||
| sql = metric["sql"] | |||
| if "calculation_method" in metric: | |||
There was a problem hiding this comment.
nit: have it inline
legacy = "calculation_method" in metric
type_ = metric["calculation_method"] if legacy else metric["type"]
sql = metric["expression"] if legacy else metric["sql"]
expression = "derived" if legacy else "expression"
|
Exciting work!! Can't wait to make some time and test this out. |
|
@betodealmeida The |
|
Ah, thanks! Let me fix it. |
Some fixes to derived metrics in dbt:
Closes #153 and #126.