Default granularity prep #1321
Default granularity prep #1321
Conversation
| @@ -570,6 +570,7 @@ class MetricSpec(InstanceSpec): # noqa: D101 | |||
| alias: Optional[str] = None | |||
| offset_window: Optional[PydanticMetricTimeWindow] = None | |||
| offset_to_grain: Optional[TimeGranularity] = None | |||
| default_granularity: Optional[TimeGranularity] = None | |||
There was a problem hiding this comment.
The fields in the spec classes should be values that can't be inferred from the semantic manifest. From get_default_granularity_for_metrics, it seems like it is?
| @@ -210,3 +210,18 @@ def get_min_queryable_time_granularity(self, metric_reference: MetricReference) | |||
| minimum_queryable_granularity = defined_time_granularity | |||
|
|
|||
| return minimum_queryable_granularity | |||
|
|
|||
| def get_default_granularity_for_metrics( | |||
There was a problem hiding this comment.
To confirm - what's the expected behavior when you have a derived metric that depends on two other metrics that have different default time granularities? Based on SetDefaultGranularityRule, it seems that the default granularity for parent metrics are not intended to be considered.
There was a problem hiding this comment.
That's correct. But now that you bring it up, I'm thinking I should confirm that behavior with product, so I'll post about it in #dev-metricflow and see what people think.
Thinking I can probably change this behavior on top of the stack if we decide to go that route.
A few changes preparing to use the
default_granularityfield to resolvemetric_timegranularity.SetDefaultGranularityRulein CLI rule setdefault_granularitytoMetricSpecMetricLookupmethod toget_default_granularity_for_metrics