Finalize validations for custom granularities

dbt_semantic_interfaces/validations/agg_time_dimension.py

@@ -28,7 +28,7 @@ def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[Validati
 
     @staticmethod
     @validate_safely(whats_being_done="checking aggregation time dimension for a semantic model")
-    def _validate_semantic_model(semantic_model: SemanticModel) -> List[ValidationIssue]:
+    def _validate_semantic_model(semantic_model: SemanticModel) -> Sequence[ValidationIssue]:
         issues: List[ValidationIssue] = []
 
         for measure in semantic_model.measures:

dbt_semantic_interfaces/validations/common_entities.py

@@ -37,7 +37,7 @@ def _check_entity(
         entity: Entity,
         semantic_model: SemanticModel,
         entities_to_semantic_models: Dict[EntityReference, Set[str]],
-    ) -> List[ValidationIssue]:
+    ) -> Sequence[ValidationIssue]:
         issues: List[ValidationIssue] = []
         # If the entity is the dict and if the set of semantic models minus this semantic model is empty,
         # then we warn the user that their entity will be unused in joins
@@ -65,15 +65,17 @@ def _check_entity(
     @validate_safely(whats_being_done="running model validation warning if entities are only one one semantic model")
     def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[ValidationIssue]:
         """Issues a warning for any entity that is associated with only one semantic_model."""
-        issues = []
+        issues: List[ValidationIssue] = []
 
         entities_to_semantic_models = CommonEntitysRule._map_semantic_model_entities(semantic_manifest.semantic_models)
         for semantic_model in semantic_manifest.semantic_models or []:
             for entity in semantic_model.entities or []:
-                issues += CommonEntitysRule._check_entity(
-                    entity=entity,
-                    semantic_model=semantic_model,
-                    entities_to_semantic_models=entities_to_semantic_models,
+                issues.extend(
+                    CommonEntitysRule._check_entity(
+                        entity=entity,
+                        semantic_model=semantic_model,
+                        entities_to_semantic_models=entities_to_semantic_models,
+                    )
                 )
 
         return issues

dbt_semantic_interfaces/validations/dimension_const.py

@@ -58,7 +58,7 @@ def _validate_dimension(
         dimension: Dimension,
         time_dims_to_granularity: Dict[DimensionReference, TimeGranularity],
         semantic_model: SemanticModel,
-    ) -> List[ValidationIssue]:
+    ) -> Sequence[ValidationIssue]:
         """Check that time dimensions of the same name and aren't primary have the same time granularity.
 
         Args:
@@ -104,7 +104,7 @@ def _validate_semantic_model(
         semantic_model: SemanticModel,
         dimension_to_invariant: Dict[DimensionReference, DimensionInvariants],
         update_invariant_dict: bool,
-    ) -> List[ValidationIssue]:
+    ) -> Sequence[ValidationIssue]:
         """Checks that the given semantic model has dimensions consistent with the given invariants.
 
         Args:

dbt_semantic_interfaces/validations/element_const.py

@@ -1,9 +1,6 @@
 from collections import defaultdict
 from typing import DefaultDict, Generic, List, Sequence
 
-from dbt_semantic_interfaces.implementations.semantic_manifest import (
-    PydanticSemanticManifest,
-)
 from dbt_semantic_interfaces.protocols import SemanticManifestT
 from dbt_semantic_interfaces.references import SemanticModelReference
 from dbt_semantic_interfaces.validations.validator_helpers import (
@@ -28,7 +25,7 @@ class ElementConsistencyRule(SemanticManifestValidationRule[SemanticManifestT],
 
     @staticmethod
     @validate_safely(whats_being_done="running model validation ensuring model wide element consistency")
-    def validate_manifest(semantic_manifest: PydanticSemanticManifest) -> Sequence[ValidationIssue]:  # noqa: D
+    def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[ValidationIssue]:  # noqa: D
         issues = []
         element_name_to_types = ElementConsistencyRule._get_element_name_to_types(semantic_manifest=semantic_manifest)
         invalid_elements = {
@@ -54,7 +51,7 @@ def validate_manifest(semantic_manifest: PydanticSemanticManifest) -> Sequence[V
 
     @staticmethod
     def _get_element_name_to_types(
-        semantic_manifest: PydanticSemanticManifest,
+        semantic_manifest: SemanticManifestT,
     ) -> DefaultDict[str, DefaultDict[SemanticModelElementType, List[SemanticModelContext]]]:
         """Create a mapping of element names in the semantic manifest to types with a list of associated contexts."""
         element_types: DefaultDict[

dbt_semantic_interfaces/validations/entities.py

@@ -26,7 +26,7 @@ class NaturalEntityConfigurationRule(SemanticManifestValidationRule[SemanticMani
             "natural entities are used in the appropriate contexts"
         )
     )
-    def _validate_semantic_model_natural_entities(semantic_model: SemanticModel) -> List[ValidationIssue]:
+    def _validate_semantic_model_natural_entities(semantic_model: SemanticModel) -> Sequence[ValidationIssue]:
         issues: List[ValidationIssue] = []
         context = SemanticModelContext(
             file_context=FileContext.from_metadata(metadata=semantic_model.metadata),

dbt_semantic_interfaces/validations/measures.py

@@ -65,7 +65,7 @@ class MeasureConstraintAliasesRule(SemanticManifestValidationRule[SemanticManife
 
     @staticmethod
     @validate_safely(whats_being_done="ensuring measures aliases are set when required")
-    def _validate_required_aliases_are_set(metric: Metric, metric_context: MetricContext) -> List[ValidationIssue]:
+    def _validate_required_aliases_are_set(metric: Metric, metric_context: MetricContext) -> Sequence[ValidationIssue]:
         """Checks if valid aliases are set on the input measure references where they are required.
 
         Aliases are required whenever there are 2 or more input measures with the same measure
@@ -188,7 +188,7 @@ class MetricMeasuresRule(SemanticManifestValidationRule[SemanticManifestT], Gene
 
     @staticmethod
     @validate_safely(whats_being_done="checking all measures referenced by the metric exist")
-    def _validate_metric_measure_references(metric: Metric, valid_measure_names: Set[str]) -> List[ValidationIssue]:
+    def _validate_metric_measure_references(metric: Metric, valid_measure_names: Set[str]) -> Sequence[ValidationIssue]:
         issues: List[ValidationIssue] = []
 
         for measure_reference in metric.measure_references:

dbt_semantic_interfaces/validations/metrics.py

@@ -120,8 +120,10 @@ def validate_manifest(cls, semantic_manifest: SemanticManifestT) -> Sequence[Val
                 )
 
             if window:
-                issues += cls.validate_metric_time_window(
-                    metric_context=metric_context, window=window, custom_granularities=custom_granularity_names
+                issues.extend(
+                    cls.validate_metric_time_window(
+                        metric_context=metric_context, window=window, custom_granularities=custom_granularity_names
+                    )
                 )
 
         return issues
@@ -167,7 +169,7 @@ class DerivedMetricRule(SemanticManifestValidationRule[SemanticManifestT], Gener
 
     @staticmethod
     @validate_safely(whats_being_done="checking that the alias set are not unique and distinct")
-    def _validate_alias_collision(metric: Metric) -> List[ValidationIssue]:
+    def _validate_alias_collision(metric: Metric) -> Sequence[ValidationIssue]:
         issues: List[ValidationIssue] = []
 
         if metric.type == MetricType.DERIVED:
@@ -193,7 +195,7 @@ def _validate_alias_collision(metric: Metric) -> List[ValidationIssue]:
 
     @staticmethod
     @validate_safely(whats_being_done="checking that the input metrics exist")
-    def _validate_input_metrics_exist(semantic_manifest: SemanticManifest) -> List[ValidationIssue]:
+    def _validate_input_metrics_exist(semantic_manifest: SemanticManifest) -> Sequence[ValidationIssue]:
         issues: List[ValidationIssue] = []
 
         all_metrics = {m.name for m in semantic_manifest.metrics}
@@ -264,7 +266,7 @@ def _validate_time_offset_params(metric: Metric, custom_granularities: Set[str])
 
     @staticmethod
     @validate_safely(whats_being_done="checking that the expr field uses the input metrics")
-    def _validate_expr(metric: Metric) -> List[ValidationIssue]:
+    def _validate_expr(metric: Metric) -> Sequence[ValidationIssue]:
         issues: List[ValidationIssue] = []
 
         if metric.type == MetricType.DERIVED:
@@ -346,7 +348,7 @@ def _validate_type_params(
     @validate_safely(whats_being_done="checks that the entity exists in the base/conversion semantic model")
     def _validate_entity_exists(
         metric: Metric, entity: str, base_semantic_model: SemanticModel, conversion_semantic_model: SemanticModel
-    ) -> List[ValidationIssue]:
+    ) -> Sequence[ValidationIssue]:
         issues: List[ValidationIssue] = []
 
         if entity not in {entity.name for entity in base_semantic_model.entities}:
@@ -376,7 +378,7 @@ def _validate_entity_exists(
     @validate_safely(whats_being_done="checks that the provided measures are valid for conversion metrics")
     def _validate_measures(
         metric: Metric, base_semantic_model: SemanticModel, conversion_semantic_model: SemanticModel
-    ) -> List[ValidationIssue]:
+    ) -> Sequence[ValidationIssue]:
         issues: List[ValidationIssue] = []
 
         def _validate_measure(
@@ -438,7 +440,7 @@ def _validate_measure(
     @validate_safely(whats_being_done="checks that the provided constant properties are valid")
     def _validate_constant_properties(
         metric: Metric, base_semantic_model: SemanticModel, conversion_semantic_model: SemanticModel
-    ) -> List[ValidationIssue]:
+    ) -> Sequence[ValidationIssue]:
         issues: List[ValidationIssue] = []
 
         def _elements_in_model(references: List[str], semantic_model: SemanticModel) -> None:

dbt_semantic_interfaces/validations/non_empty.py

@@ -17,7 +17,7 @@ class NonEmptyRule(SemanticManifestValidationRule[SemanticManifestT], Generic[Se
 
     @staticmethod
     @validate_safely(whats_being_done="checking that the model has semantic models")
-    def _check_model_has_semantic_models(semantic_manifest: PydanticSemanticManifest) -> List[ValidationIssue]:
+    def _check_model_has_semantic_models(semantic_manifest: PydanticSemanticManifest) -> Sequence[ValidationIssue]:
         issues: List[ValidationIssue] = []
         if not semantic_manifest.semantic_models:
             issues.append(
@@ -29,7 +29,7 @@ def _check_model_has_semantic_models(semantic_manifest: PydanticSemanticManifest
 
     @staticmethod
     @validate_safely(whats_being_done="checking that the model has metrics")
-    def _check_model_has_metrics(semantic_manifest: PydanticSemanticManifest) -> List[ValidationIssue]:
+    def _check_model_has_metrics(semantic_manifest: PydanticSemanticManifest) -> Sequence[ValidationIssue]:
         issues: List[ValidationIssue] = []
 
         # If we are going to generate measure proxy metrics that is sufficient as well
@@ -50,8 +50,12 @@ def _check_model_has_metrics(semantic_manifest: PydanticSemanticManifest) -> Lis
 
     @staticmethod
     @validate_safely("running model validation rule ensuring metrics and semantic models are defined")
-    def validate_manifest(semantic_manifest: PydanticSemanticManifest) -> Sequence[ValidationIssue]:  # noqa: D
+    def validate_manifest(  # noqa: D
+        # PydanticSemanticManifest is required here due to a Measure.create_metric call downstream.
+        # TODO: can we add create_metric to the Measure protocol to avoid this type override?
+        semantic_manifest: PydanticSemanticManifest,  # type: ignore[override]
+    ) -> Sequence[ValidationIssue]:
         issues: List[ValidationIssue] = []
-        issues += NonEmptyRule._check_model_has_semantic_models(semantic_manifest=semantic_manifest)
-        issues += NonEmptyRule._check_model_has_metrics(semantic_manifest=semantic_manifest)
+        issues.extend(NonEmptyRule._check_model_has_semantic_models(semantic_manifest=semantic_manifest))
+        issues.extend(NonEmptyRule._check_model_has_metrics(semantic_manifest=semantic_manifest))
         return issues

dbt_semantic_interfaces/validations/reserved_keywords.py

@@ -1,4 +1,4 @@
-from typing import Generic, List
+from typing import Generic, List, Sequence
 
 from dbt_semantic_interfaces.protocols import (
     SemanticManifest,
@@ -67,7 +67,7 @@ class ReservedKeywordsRule(SemanticManifestValidationRule[SemanticManifestT], Ge
 
     @staticmethod
     @validate_safely(whats_being_done="checking that semantic model sub element names aren't reserved sql keywords")
-    def _validate_semantic_model_sub_elements(semantic_model: SemanticModel) -> List[ValidationIssue]:
+    def _validate_semantic_model_sub_elements(semantic_model: SemanticModel) -> Sequence[ValidationIssue]:
         issues: List[ValidationIssue] = []
 
         for dimension in semantic_model.dimensions:
@@ -125,7 +125,7 @@ def _validate_semantic_model_sub_elements(semantic_model: SemanticModel) -> List
 
     @classmethod
     @validate_safely(whats_being_done="checking that semantic_model node_relations are not sql reserved keywords")
-    def _validate_semantic_models(cls, semantic_manifest: SemanticManifest) -> List[ValidationIssue]:
+    def _validate_semantic_models(cls, semantic_manifest: SemanticManifest) -> Sequence[ValidationIssue]:
         """Checks names of objects that are not nested."""
         issues: List[ValidationIssue] = []
         set_keywords = set(RESERVED_KEYWORDS)
@@ -156,5 +156,5 @@ def _validate_semantic_models(cls, semantic_manifest: SemanticManifest) -> List[
         whats_being_done="running model validation ensuring elements that aren't selected via a defined expr don't "
         "contain reserved keywords"
     )
-    def validate_manifest(cls, semantic_manifest: SemanticManifest) -> List[ValidationIssue]:  # noqa: D
+    def validate_manifest(cls, semantic_manifest: SemanticManifest) -> Sequence[ValidationIssue]:  # noqa: D
         return cls._validate_semantic_models(semantic_manifest=semantic_manifest)

dbt_semantic_interfaces/validations/semantic_models.py

@@ -36,7 +36,7 @@ def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[Validati
         whats_being_done="checking the semantic model's validity parameters for compatibility with "
         "runtime requirements"
     )
-    def _validate_semantic_model(semantic_model: SemanticModel) -> List[ValidationIssue]:
+    def _validate_semantic_model(semantic_model: SemanticModel) -> Sequence[ValidationIssue]:
         """Runs assertions on semantic models with validity parameters set on one or more time dimensions."""
         issues: List[ValidationIssue] = []
 
@@ -157,7 +157,7 @@ def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[Validati
 
     @staticmethod
     @validate_safely(whats_being_done="checking validity of the semantic model's default agg_time_dimension")
-    def _validate_default_agg_time_dimension(semantic_model: SemanticModel) -> List[ValidationIssue]:
+    def _validate_default_agg_time_dimension(semantic_model: SemanticModel) -> Sequence[ValidationIssue]:
         issues: List[ValidationIssue] = []
 
         if semantic_model.defaults is None or semantic_model.defaults.agg_time_dimension is None:

dbt_semantic_interfaces/validations/unique_valid_name.py

@@ -69,7 +69,9 @@ class UniqueAndValidNameRule(SemanticManifestValidationRule[SemanticManifestT],
     NAME_REGEX = re.compile(r"\A[a-z]((?!__)[a-z0-9_])*[a-z0-9]\Z")
 
     @staticmethod
-    def check_valid_name(name: str, context: Optional[ValidationContext] = None) -> List[ValidationIssue]:  # noqa: D
+    def check_valid_name(  # noqa: D
+        name: str, context: Optional[ValidationContext] = None
+    ) -> Sequence[ValidationIssue]:
         issues: List[ValidationIssue] = []
 
         if not UniqueAndValidNameRule.NAME_REGEX.match(name):
@@ -102,7 +104,9 @@ def check_valid_name(name: str, context: Optional[ValidationContext] = None) ->
 
     @staticmethod
     @validate_safely(whats_being_done="checking semantic model sub element names are unique")
-    def _validate_semantic_model_elements_and_time_spines(semantic_manifest: SemanticManifest) -> List[ValidationIssue]:
+    def _validate_semantic_model_elements_and_time_spines(
+        semantic_manifest: SemanticManifest,
+    ) -> Sequence[ValidationIssue]:
         issues: List[ValidationIssue] = []
         custom_granularity_restricted_names_and_types: Dict[str, str] = {}
 
@@ -219,9 +223,9 @@ def _validate_semantic_model_elements_and_time_spines(semantic_manifest: Semanti
     @staticmethod
     @validate_safely(whats_being_done="checking top level elements of a specific type have unique and valid names")
     def _validate_top_level_objects_of_type(
-        objects: Union[List[SemanticModel], List[Metric], List[SavedQuery]],
+        objects: Union[Sequence[SemanticModel], Sequence[Metric], Sequence[SavedQuery]],
         object_type: SemanticManifestNodeType,
-    ) -> List[ValidationIssue]:
+    ) -> Sequence[ValidationIssue]:
         """Validates uniqeness and validaty of top level objects of singular type."""
         issues: List[ValidationIssue] = []
         object_names = set()
@@ -247,11 +251,9 @@ def _validate_top_level_objects_of_type(
 
     @staticmethod
     @validate_safely(whats_being_done="checking model top level element names are sufficiently unique")
-    def _validate_top_level_objects(semantic_manifest: SemanticManifest) -> List[ValidationIssue]:
+    def _validate_top_level_objects(semantic_manifest: SemanticManifest) -> Sequence[ValidationIssue]:
         """Checks names of objects that are not nested."""
-        issues: List[ValidationIssue] = []
-
-        issues.extend(
+        issues = list(
             UniqueAndValidNameRule._validate_top_level_objects_of_type(
                 semantic_manifest.semantic_models, SemanticManifestNodeType.SEMANTIC_MODEL
             )
@@ -274,7 +276,7 @@ def _validate_top_level_objects(semantic_manifest: SemanticManifest) -> List[Val
     @staticmethod
     @validate_safely(whats_being_done="running model validation ensuring elements have adequately unique names")
     def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[ValidationIssue]:  # noqa: D
-        issues = []
+        issues: List[ValidationIssue] = []
         issues += UniqueAndValidNameRule._validate_top_level_objects(semantic_manifest=semantic_manifest)
         issues += UniqueAndValidNameRule._validate_semantic_model_elements_and_time_spines(semantic_manifest)
 
@@ -336,7 +338,7 @@ def _check_semantic_model(  # noqa: D
     @staticmethod
     @validate_safely(whats_being_done="validating there are no duplicate dimension primary entity pairs")
     def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[ValidationIssue]:  # noqa: D
-        issues = []
+        issues: List[ValidationIssue] = []
         known_pairings: Dict[str, Dict[str, str]] = {}
         for semantic_model in semantic_manifest.semantic_models:
             issues += PrimaryEntityDimensionPairs._check_semantic_model(

tests/validations/test_validator_helpers.py

@@ -1,5 +1,5 @@
 from datetime import date
-from typing import List
+from typing import List, Sequence
 
 import pytest
 
@@ -25,7 +25,7 @@
 
 
 @pytest.fixture
-def list_of_issues() -> List[ValidationIssue]:  # noqa: D
+def list_of_issues() -> Sequence[ValidationIssue]:  # noqa: D
     file_context = FileContext(file_name="foo", line_number=1337)
     semantic_model_name = "My semantic model"
 
@@ -141,7 +141,7 @@ def test_merge_two_model_validation_results(list_of_issues: List[ValidationIssue
 
 def test_validate_safely_handles_exceptions():  # noqa: D
     @validate_safely("testing validate safely handles exceptions gracefully")
-    def checking_validate_safely() -> List[ValidationIssue]:
+    def checking_validate_safely() -> Sequence[ValidationIssue]:
         raise (Exception("Oh no an exception!"))
         return []