Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch from checked_agg_time_dimension to checked_agg_time_dimension_for_measure #113

Merged
merged 10 commits into from
Jul 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changes/unreleased/Breaking Changes-20230711-162541.yaml
Original file line number Diff line number Diff line change
@@ -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
11 changes: 1 addition & 10 deletions dbt_semantic_interfaces/implementations/elements/measure.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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)
14 changes: 14 additions & 0 deletions dbt_semantic_interfaces/implementations/semantic_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
LinkableElementReference,
MeasureReference,
SemanticModelReference,
TimeDimensionReference,
)


Expand Down Expand Up @@ -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)
8 changes: 1 addition & 7 deletions dbt_semantic_interfaces/protocols/measure.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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:
Expand Down
9 changes: 9 additions & 0 deletions dbt_semantic_interfaces/protocols/semantic_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
LinkableElementReference,
MeasureReference,
SemanticModelReference,
TimeDimensionReference,
)


Expand Down Expand Up @@ -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)
36 changes: 0 additions & 36 deletions dbt_semantic_interfaces/transformations/agg_time_dimension.py

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
):
Expand Down
9 changes: 3 additions & 6 deletions dbt_semantic_interfaces/validations/measures.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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}'."
),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -62,7 +61,6 @@ class SemanticManifestValidator(Generic[SemanticManifestT]):
DerivedMetricRule[SemanticManifestT](),
CountAggregationExprRule[SemanticManifestT](),
SemanticModelMeasuresUniqueRule[SemanticManifestT](),
SemanticModelTimeDimensionWarningsRule[SemanticManifestT](),
SemanticModelValidityWindowRule[SemanticManifestT](),
DimensionConsistencyRule[SemanticManifestT](),
ElementConsistencyRule[SemanticManifestT](),
Expand Down
37 changes: 0 additions & 37 deletions dbt_semantic_interfaces/validations/semantic_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""

Expand Down
3 changes: 3 additions & 0 deletions tests/validations/test_agg_time_dimension.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down