diff --git a/.changes/unreleased/Breaking Changes-20230711-162541.yaml b/.changes/unreleased/Breaking Changes-20230711-162541.yaml new file mode 100644 index 00000000..eae84a16 --- /dev/null +++ b/.changes/unreleased/Breaking Changes-20230711-162541.yaml @@ -0,0 +1,6 @@ +kind: Breaking Changes +body: Migrate from `checked_agg_time_dimension` to `checked_agg_time_dimension_for_measure` +time: 2023-07-11T16:25:41.337984-07:00 +custom: + Author: QMalcolm + Issue: None diff --git a/dbt_semantic_interfaces/implementations/elements/measure.py b/dbt_semantic_interfaces/implementations/elements/measure.py index 48313719..1036e1b2 100644 --- a/dbt_semantic_interfaces/implementations/elements/measure.py +++ b/dbt_semantic_interfaces/implementations/elements/measure.py @@ -7,7 +7,7 @@ ModelWithMetadataParsing, ) from dbt_semantic_interfaces.implementations.metadata import PydanticMetadata -from dbt_semantic_interfaces.references import MeasureReference, TimeDimensionReference +from dbt_semantic_interfaces.references import MeasureReference from dbt_semantic_interfaces.type_enums import AggregationType @@ -45,15 +45,6 @@ class PydanticMeasure(HashableBaseModel, ModelWithMetadataParsing): non_additive_dimension: Optional[PydanticNonAdditiveDimensionParameters] = None agg_time_dimension: Optional[str] = None - @property - def checked_agg_time_dimension(self) -> TimeDimensionReference: # noqa: D - assert self.agg_time_dimension, ( - f"Aggregation time dimension for measure {self.name} is not set! This should either be set directly on " - f"the measure specification in the model, or else defaulted to the primary time dimension in the data " - f"source containing the measure." - ) - return TimeDimensionReference(element_name=self.agg_time_dimension) - @property def reference(self) -> MeasureReference: # noqa: D return MeasureReference(element_name=self.name) diff --git a/dbt_semantic_interfaces/implementations/semantic_model.py b/dbt_semantic_interfaces/implementations/semantic_model.py index 94862ae9..222a5902 100644 --- a/dbt_semantic_interfaces/implementations/semantic_model.py +++ b/dbt_semantic_interfaces/implementations/semantic_model.py @@ -22,6 +22,7 @@ LinkableElementReference, MeasureReference, SemanticModelReference, + TimeDimensionReference, ) @@ -167,3 +168,16 @@ def get_entity(self, entity_reference: LinkableElementReference) -> PydanticEnti return entity raise ValueError(f"No entity with name ({entity_reference}) in semantic_model with name ({self.name})") + + def checked_agg_time_dimension_for_measure(self, measure_reference: MeasureReference): # noqa: D + measure = self.get_measure(measure_reference=measure_reference) + if self.defaults is not None: + default_agg_time_dimesion = self.defaults.agg_time_dimension + + agg_time_dimension_name = measure.agg_time_dimension or default_agg_time_dimesion + assert agg_time_dimension_name is not None, ( + f"Aggregation time dimension for measure {measure.name} is not set! This should either be set directly on " + f"the measure specification in the model, or else defaulted to the primary time dimension in the data " + f"source containing the measure." + ) + return TimeDimensionReference(element_name=agg_time_dimension_name) diff --git a/dbt_semantic_interfaces/protocols/measure.py b/dbt_semantic_interfaces/protocols/measure.py index 6a35f405..aabdf355 100644 --- a/dbt_semantic_interfaces/protocols/measure.py +++ b/dbt_semantic_interfaces/protocols/measure.py @@ -3,7 +3,7 @@ from abc import abstractmethod from typing import Optional, Protocol, Sequence -from dbt_semantic_interfaces.references import MeasureReference, TimeDimensionReference +from dbt_semantic_interfaces.references import MeasureReference from dbt_semantic_interfaces.type_enums import AggregationType @@ -87,12 +87,6 @@ def non_additive_dimension(self) -> Optional[NonAdditiveDimensionParameters]: # def agg_time_dimension(self) -> Optional[str]: # noqa: D pass - @property - @abstractmethod - def checked_agg_time_dimension(self) -> TimeDimensionReference: - """Returns the aggregation time dimension, throwing an exception if it's not set.""" - ... - @property @abstractmethod def reference(self) -> MeasureReference: diff --git a/dbt_semantic_interfaces/protocols/semantic_model.py b/dbt_semantic_interfaces/protocols/semantic_model.py index 9bc4f133..bca4999d 100644 --- a/dbt_semantic_interfaces/protocols/semantic_model.py +++ b/dbt_semantic_interfaces/protocols/semantic_model.py @@ -11,6 +11,7 @@ LinkableElementReference, MeasureReference, SemanticModelReference, + TimeDimensionReference, ) @@ -146,5 +147,13 @@ def reference(self) -> SemanticModelReference: def metadata(self) -> Optional[Metadata]: # noqa: D pass + @abstractmethod + def checked_agg_time_dimension_for_measure(self, measure_reference: MeasureReference) -> TimeDimensionReference: + """Returns the `TimeDimensionReference` what a measure should use for it's `agg_time_dimension`. + + Should raise an exception if a TimeDimensionReference cannot be built + """ + ... + SemanticModelT = TypeVar("SemanticModelT", bound=SemanticModel) diff --git a/dbt_semantic_interfaces/transformations/agg_time_dimension.py b/dbt_semantic_interfaces/transformations/agg_time_dimension.py deleted file mode 100644 index 011978a6..00000000 --- a/dbt_semantic_interfaces/transformations/agg_time_dimension.py +++ /dev/null @@ -1,36 +0,0 @@ -import logging - -from typing_extensions import override - -from dbt_semantic_interfaces.implementations.semantic_manifest import ( - PydanticSemanticManifest, -) -from dbt_semantic_interfaces.protocols import ProtocolHint -from dbt_semantic_interfaces.transformations.transform_rule import ( - SemanticManifestTransformRule, -) - -logger = logging.getLogger(__name__) - - -class SetMeasureAggregationTimeDimensionRule(ProtocolHint[SemanticManifestTransformRule[PydanticSemanticManifest]]): - """Sets the aggregation time dimension for measures to the one specified as default.""" - - @override - def _implements_protocol(self) -> SemanticManifestTransformRule[PydanticSemanticManifest]: # noqa: D - return self - - @staticmethod - def transform_model(semantic_manifest: PydanticSemanticManifest) -> PydanticSemanticManifest: # noqa: D - for semantic_model in semantic_manifest.semantic_models: - if semantic_model.defaults is None: - continue - - if semantic_model.defaults.agg_time_dimension is None: - continue - - for measure in semantic_model.measures: - if not measure.agg_time_dimension: - measure.agg_time_dimension = semantic_model.defaults.agg_time_dimension - - return semantic_manifest diff --git a/dbt_semantic_interfaces/transformations/pydantic_rule_set.py b/dbt_semantic_interfaces/transformations/pydantic_rule_set.py index 2e772005..bf4ff96e 100644 --- a/dbt_semantic_interfaces/transformations/pydantic_rule_set.py +++ b/dbt_semantic_interfaces/transformations/pydantic_rule_set.py @@ -10,9 +10,6 @@ from dbt_semantic_interfaces.transformations.add_input_metric_measures import ( AddInputMetricMeasuresRule, ) -from dbt_semantic_interfaces.transformations.agg_time_dimension import ( - SetMeasureAggregationTimeDimensionRule, -) from dbt_semantic_interfaces.transformations.boolean_measure import ( BooleanMeasureAggregationRule, ) @@ -43,10 +40,7 @@ def _implements_protocol(self) -> SemanticManifestTransformRuleSet[PydanticSeman @property def primary_rules(self) -> Sequence[SemanticManifestTransformRule[PydanticSemanticManifest]]: # noqa: - return ( - LowerCaseNamesRule(), - SetMeasureAggregationTimeDimensionRule(), - ) + return (LowerCaseNamesRule(),) @property def secondary_rules(self) -> Sequence[SemanticManifestTransformRule[PydanticSemanticManifest]]: # noqa: D diff --git a/dbt_semantic_interfaces/validations/agg_time_dimension.py b/dbt_semantic_interfaces/validations/agg_time_dimension.py index cf609b2d..0d21a55b 100644 --- a/dbt_semantic_interfaces/validations/agg_time_dimension.py +++ b/dbt_semantic_interfaces/validations/agg_time_dimension.py @@ -39,7 +39,7 @@ def _validate_semantic_model(semantic_model: SemanticModel) -> List[ValidationIs ), element_type=SemanticModelElementType.MEASURE, ) - agg_time_dimension_reference = measure.checked_agg_time_dimension + agg_time_dimension_reference = semantic_model.checked_agg_time_dimension_for_measure(measure.reference) if not SemanticModelValidationHelpers.time_dimension_in_model( time_dimension_name=agg_time_dimension_reference.element_name, semantic_model=semantic_model ): diff --git a/dbt_semantic_interfaces/validations/measures.py b/dbt_semantic_interfaces/validations/measures.py index e579cc89..94a72981 100644 --- a/dbt_semantic_interfaces/validations/measures.py +++ b/dbt_semantic_interfaces/validations/measures.py @@ -232,12 +232,9 @@ def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[Validati non_additive_dimension = measure.non_additive_dimension if non_additive_dimension is None: continue + agg_time_dimension_reference = semantic_model.checked_agg_time_dimension_for_measure(measure.reference) agg_time_dimension = next( - ( - dim - for dim in semantic_model.dimensions - if measure.checked_agg_time_dimension.element_name == dim.name - ), + (dim for dim in semantic_model.dimensions if agg_time_dimension_reference.element_name == dim.name), None, ) if agg_time_dimension is None: @@ -253,7 +250,7 @@ def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[Validati ), message=( f"Measure '{measure.name}' has a agg_time_dimension of " - f"{measure.checked_agg_time_dimension.element_name} " + f"{agg_time_dimension_reference.element_name} " f"that is not defined as a dimension in semantic model '{semantic_model.name}'." ), ) diff --git a/dbt_semantic_interfaces/validations/semantic_manifest_validator.py b/dbt_semantic_interfaces/validations/semantic_manifest_validator.py index b5edf2d9..de11a5a6 100644 --- a/dbt_semantic_interfaces/validations/semantic_manifest_validator.py +++ b/dbt_semantic_interfaces/validations/semantic_manifest_validator.py @@ -29,7 +29,6 @@ from dbt_semantic_interfaces.validations.reserved_keywords import ReservedKeywordsRule from dbt_semantic_interfaces.validations.semantic_models import ( SemanticModelDefaultsRule, - SemanticModelTimeDimensionWarningsRule, SemanticModelValidityWindowRule, ) from dbt_semantic_interfaces.validations.unique_valid_name import UniqueAndValidNameRule @@ -62,7 +61,6 @@ class SemanticManifestValidator(Generic[SemanticManifestT]): DerivedMetricRule[SemanticManifestT](), CountAggregationExprRule[SemanticManifestT](), SemanticModelMeasuresUniqueRule[SemanticManifestT](), - SemanticModelTimeDimensionWarningsRule[SemanticManifestT](), SemanticModelValidityWindowRule[SemanticManifestT](), DimensionConsistencyRule[SemanticManifestT](), ElementConsistencyRule[SemanticManifestT](), diff --git a/dbt_semantic_interfaces/validations/semantic_models.py b/dbt_semantic_interfaces/validations/semantic_models.py index dd931383..0241131d 100644 --- a/dbt_semantic_interfaces/validations/semantic_models.py +++ b/dbt_semantic_interfaces/validations/semantic_models.py @@ -17,43 +17,6 @@ logger = logging.getLogger(__name__) -class SemanticModelTimeDimensionWarningsRule( - SemanticManifestValidationRule[SemanticManifestT], Generic[SemanticManifestT] -): - """Checks time dimensions in semantic models.""" - - @staticmethod - @validate_safely(whats_being_done="running model validation ensuring time dimensions are defined properly") - def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[ValidationIssue]: # noqa: D - issues: List[ValidationIssue] = [] - - for semantic_model in semantic_manifest.semantic_models: - issues.extend( - SemanticModelTimeDimensionWarningsRule._validate_semantic_model(semantic_model=semantic_model) - ) - return issues - - @staticmethod - @validate_safely(whats_being_done="checking validity of the semantic model's time dimensions") - def _validate_semantic_model(semantic_model: SemanticModel) -> List[ValidationIssue]: - issues: List[ValidationIssue] = [] - - for measure in semantic_model.measures: - if measure.agg_time_dimension is None: - issues.append( - ValidationError( - context=SemanticModelContext( - file_context=FileContext.from_metadata(metadata=semantic_model.metadata), - semantic_model=SemanticModelReference(semantic_model_name=semantic_model.name), - ), - message=f"Aggregation time dimension not specified for measure named '{measure.name}' in the " - f"semantic model named '{semantic_model.name}'. Please add one", - ) - ) - - return issues - - class SemanticModelValidityWindowRule(SemanticManifestValidationRule[SemanticManifestT], Generic[SemanticManifestT]): """Checks validity windows in semantic models to ensure they comply with runtime requirements.""" diff --git a/tests/validations/test_agg_time_dimension.py b/tests/validations/test_agg_time_dimension.py index 223586f8..c87dca1e 100644 --- a/tests/validations/test_agg_time_dimension.py +++ b/tests/validations/test_agg_time_dimension.py @@ -45,6 +45,9 @@ def test_unset_aggregation_time_dimension(simple_semantic_manifest: PydanticSema lambda semantic_model: len(semantic_model.measures) > 0, ) + if semantic_model_with_measures.defaults is not None: + semantic_model_with_measures.defaults.agg_time_dimension = None + semantic_model_with_measures.measures[0].agg_time_dimension = None with pytest.raises(