Use hypothetical get_metric() contextmember to remove gnarliest code
#11
Conversation
There was a problem hiding this comment.
Big fan 🤩 There's not much to comment on over this side, other than whether the calendar override should be a ref or a string
I don't feel too strongly about this.
By contrast, I feel reasonably strongly that it should be a ref - discussed on the line of code itself
| {% macro get_metric(metric_name) %} | ||
| {% if not execute %} | ||
| {% do return(None) %} | ||
| {% else %} | ||
| {% set metric_info = namespace(metric_id=none) %} | ||
| {% for metric in graph.metrics.values() %} | ||
| {% if metric.name == metric_name %} | ||
| {% set metric_info.metric_id = metric.unique_id %} | ||
| {% endif %} | ||
| {% endfor %} | ||
|
|
||
| {% if metric_info.metric_id is none %} | ||
| {% do exceptions.raise_compiler_error("Metric named '" ~ metric_name ~ "' not found") %} | ||
| {% endif %} | ||
|
|
||
|
|
||
| {% do return(graph.metrics[metric_info.metric_id]) %} | ||
| {% endif %} | ||
|
|
||
| {% endmacro %} |
| @@ -114,7 +115,7 @@ To do this, set the value of the `dbt_metrics_calendar_model` variable in your ` | |||
| config-version: 2 | |||
| [...] | |||
| vars: | |||
| dbt_metrics_calendar_model: ref('my_custom_table') | |||
| dbt_metrics_calendar_model: my_custom_table # BREAKING change: just the model name, no ref | |||
There was a problem hiding this comment.
This does make this very ergonomic, but it would be an outlier in terms of how all other refs are handled (cf relationships tests, other packages and metrics themselves) so I would prefer it to stay as a ref instead of an outlier if possible
|
@jtcohen6 I believe we spoke about this and this PR should be closed out due to us moving forward with the If you can confirm I'll close this one out. |
|
@callum-mcdata Yes, that's my hope & aspiration! |
Try
get_metric()contextmember added in dbt-labs/dbt-core@bd6a4a1, as proposed/discussed in dbt-labs/dbt-core#4641 (comment)This built-in
get_metric()context member / macro:graph) entry for the metricmodelfrom a string ("ref(my_model)") to the actual relation thatref()call would returnI also switched the
dbt_metrics_calendar_modelvariable to return the string name of a model, rather than a string containingref(), because it was very slightly more convenient to write the calendarref(). I don't feel too strongly about this.