From 31a8e2427c33d69b35c5be5faacd77373e70da13 Mon Sep 17 00:00:00 2001 From: Courtney Holcomb Date: Mon, 10 Jun 2024 12:02:16 -0700 Subject: [PATCH] WIP --- .../implementations/metric.py | 2 +- .../default_explicit_schema.json | 39 +++++++++++++ dbt_semantic_interfaces/parsing/schemas.py | 16 ++++++ dbt_semantic_interfaces/protocols/metric.py | 25 ++++++++ .../validations/metrics.py | 57 +++++++++++++++---- .../simple_semantic_manifest/metrics.yaml | 7 ++- 6 files changed, 131 insertions(+), 15 deletions(-) diff --git a/dbt_semantic_interfaces/implementations/metric.py b/dbt_semantic_interfaces/implementations/metric.py index 72a2610d..7fdf2ce0 100644 --- a/dbt_semantic_interfaces/implementations/metric.py +++ b/dbt_semantic_interfaces/implementations/metric.py @@ -164,7 +164,7 @@ class PydanticCumulativeTypeParams(HashableBaseModel): window: Optional[PydanticMetricTimeWindow] grain_to_date: Optional[TimeGranularity] - period_agg: Optional[PeriodAggregation] = PeriodAggregation.START + period_agg: Optional[PeriodAggregation] class PydanticMetricTypeParams(HashableBaseModel): diff --git a/dbt_semantic_interfaces/parsing/generated_json_schemas/default_explicit_schema.json b/dbt_semantic_interfaces/parsing/generated_json_schemas/default_explicit_schema.json index 38047288..518a03d0 100644 --- a/dbt_semantic_interfaces/parsing/generated_json_schemas/default_explicit_schema.json +++ b/dbt_semantic_interfaces/parsing/generated_json_schemas/default_explicit_schema.json @@ -72,6 +72,42 @@ ], "type": "object" }, + "cumulative_type_params_schema": { + "$id": "cumulative_type_params_schema", + "additionalProperties": false, + "properties": { + "window": {"type": "string"}, + "grain_to_date": { + "enum": [ + "NANOSECOND", + "MICROSECOND", + "MILLISECOND", + "SECOND", + "MINUTE", + "HOUR", + "DAY", + "WEEK", + "MONTH", + "QUARTER", + "YEAR", + "nanosecond", + "microsecond", + "millisecond", + "second", + "minute", + "hour", + "day", + "week", + "month", + "quarter", + "year" + ] + }, + "period_agg": {"enum": ["START", "END", "AVERAGE", "start", "end", "average"]} + }, + "required": [], + "type": "object" + }, "dimension_schema": { "$id": "dimension_schema", "additionalProperties": false, @@ -456,6 +492,9 @@ "conversion_type_params": { "$ref": "#/definitions/conversion_type_params_schema" }, + "cumulative_type_params_schema": { + "$ref": "#/definitions/cumulative_type_params_schema" + }, "denominator": { "$ref": "#/definitions/metric_input_measure_schema" }, diff --git a/dbt_semantic_interfaces/parsing/schemas.py b/dbt_semantic_interfaces/parsing/schemas.py index 78a60f3e..46edbb88 100644 --- a/dbt_semantic_interfaces/parsing/schemas.py +++ b/dbt_semantic_interfaces/parsing/schemas.py @@ -57,6 +57,9 @@ export_destination_type_values = ["TABLE", "VIEW"] export_destination_type_values += [x.lower() for x in export_destination_type_values] +period_agg_values = ["START", "END", "AVERAGE"] +period_agg_values += [x.lower() for x in period_agg_values] + filter_schema = { "$id": "filter_schema", @@ -115,6 +118,18 @@ "required": ["base_measure", "conversion_measure", "entity"], } +cumulative_type_params_schema = { + "$id": "cumulative_type_params_schema", + "type": "object", + "properties": { + "window": {"type": "string"}, + "grain_to_date": {"enum": time_granularity_values}, + "period_agg": {"enum": period_agg_values}, + }, + "additionalProperties": False, + "required": [], +} + constant_property_input_schema = { "$id": "constant_property_input_schema", "type": "object", @@ -141,6 +156,7 @@ "items": {"$ref": "metric_input_schema"}, }, "conversion_type_params": {"$ref": "conversion_type_params_schema"}, + "cumulative_type_params": {"$ref": "cumulative_type_params_schema"}, }, "additionalProperties": False, } diff --git a/dbt_semantic_interfaces/protocols/metric.py b/dbt_semantic_interfaces/protocols/metric.py index f3ac65dd..71925e49 100644 --- a/dbt_semantic_interfaces/protocols/metric.py +++ b/dbt_semantic_interfaces/protocols/metric.py @@ -9,6 +9,7 @@ from dbt_semantic_interfaces.type_enums import ( ConversionCalculationType, MetricType, + PeriodAggregation, TimeGranularity, ) @@ -178,6 +179,25 @@ def constant_properties(self) -> Optional[Sequence[ConstantPropertyInput]]: pass +class CumulativeTypeParams(Protocol): + """Type params to provide context for cumulative metric properties.""" + + @property + @abstractmethod + def window(self) -> Optional[MetricTimeWindow]: # noqa: D + pass + + @property + @abstractmethod + def grain_to_date(self) -> Optional[TimeGranularity]: # noqa: D + pass + + @property + @abstractmethod + def period_agg(self) -> Optional[PeriodAggregation]: # noqa: D + pass + + class MetricTypeParams(Protocol): """Type params add additional context to certain metric types (the context depends on the metric type).""" @@ -227,6 +247,11 @@ def metrics(self) -> Optional[Sequence[MetricInput]]: # noqa: D def conversion_type_params(self) -> Optional[ConversionTypeParams]: # noqa: D pass + @property + @abstractmethod + def cumulative_type_params(self) -> Optional[CumulativeTypeParams]: # noqa: D + pass + class MetricConfig(Protocol): # noqa: D """The config property allows you to configure additional resources/metadata.""" diff --git a/dbt_semantic_interfaces/validations/metrics.py b/dbt_semantic_interfaces/validations/metrics.py index 4533a1df..299a2801 100644 --- a/dbt_semantic_interfaces/validations/metrics.py +++ b/dbt_semantic_interfaces/validations/metrics.py @@ -19,6 +19,7 @@ SemanticManifestValidationRule, ValidationError, ValidationIssue, + ValidationWarning, generate_exception_issue, validate_safely, ) @@ -33,29 +34,61 @@ def _validate_cumulative_sum_metric_params(metric: Metric) -> List[ValidationIss issues: List[ValidationIssue] = [] if metric.type == MetricType.CUMULATIVE: - if metric.type_params.window and metric.type_params.grain_to_date: + metric_context = MetricContext( + file_context=FileContext.from_metadata(metadata=metric.metadata), + metric=MetricModelReference(metric_name=metric.name), + ) + + for field in ("window", "grain_to_date"): + if getattr(metric.type_params, field): + issues.append( + ValidationWarning( + context=metric_context, + message=( + f"Cumulative `type_params.{field}` field has been moved and will soon be deprecated. " + f"Please nest that value under `type_params.cumulative_type_params.{field}`." + ), + ) + ) + + if ( + getattr(metric.type_params, field) + and metric.type_params.cumulative_type_params + and getattr(metric.type_params.cumulative_type_params, field) + ): + issues.append( + ValidationWarning( + context=metric_context, + message=( + f"`{field}` set twice in cumulative metric '{metric.name}'. The value set in " + "`cumulative_type_params` will be used. Please remove it from `type_params`." + ), + ) + ) + + window = metric.type_params.window + if metric.type_params.cumulative_type_params: + window = metric.type_params.cumulative_type_params.window + grain_to_date = metric.type_params.grain_to_date + if metric.type_params.cumulative_type_params: + grain_to_date = metric.type_params.cumulative_type_params.grain_to_date + if window and grain_to_date: issues.append( ValidationError( - context=MetricContext( - file_context=FileContext.from_metadata(metadata=metric.metadata), - metric=MetricModelReference(metric_name=metric.name), - ), - message="Both window and grain_to_date set for cumulative metric. Please set one or the other", + context=metric_context, + message="Both window and grain_to_date set for cumulative metric. Please set one or the other.", ) ) - if metric.type_params.window: + if window: try: - window_str = f"{metric.type_params.window.count} {metric.type_params.window.granularity.value}" + window_str = f"{window.count} {window.granularity.value}" # TODO: Should not call an implementation class. PydanticMetricTimeWindow.parse(window_str) except ParsingException as e: issues.append( ValidationError( - context=MetricContext( - file_context=FileContext.from_metadata(metadata=metric.metadata), - metric=MetricModelReference(metric_name=metric.name), - ), + context=metric_context, message="".join(traceback.format_exception_only(type(e), value=e)), extra_detail="".join(traceback.format_tb(e.__traceback__)), ) diff --git a/tests/fixtures/semantic_manifest_yamls/simple_semantic_manifest/metrics.yaml b/tests/fixtures/semantic_manifest_yamls/simple_semantic_manifest/metrics.yaml index 9b834a34..78c2acb7 100644 --- a/tests/fixtures/semantic_manifest_yamls/simple_semantic_manifest/metrics.yaml +++ b/tests/fixtures/semantic_manifest_yamls/simple_semantic_manifest/metrics.yaml @@ -174,7 +174,9 @@ metric: type_params: measure: name: bookers - window: 2 days + cumulative_type_params: + window: 2 days + period_agg: avg --- metric: name: "revenue_mtd" @@ -183,7 +185,8 @@ metric: type_params: measure: name: txn_revenue - grain_to_date: month + cumulative_type_params: + grain_to_date: month --- metric: name: booking_fees