Making Metric Timestamps Optional (#6402)

* changing to optional

* adding tests

* tests and changie

* pre-commit cleaning

* formatting fixes

* pre-commit update

.changes/unreleased/Features-20221207-091722.yaml

@@ -0,0 +1,7 @@
+kind: Features
+body: Making timestamp optional for metrics
+time: 2022-12-07T09:17:22.571877-06:00
+custom:
+  Author: callum-mcdata
+  Issue: "6398"
+  PR: "9400"

core/dbt/contracts/graph/nodes.py

@@ -976,12 +976,12 @@ class Metric(GraphNode):
     description: str
     label: str
     calculation_method: str
-    timestamp: str
     expression: str
     filters: List[MetricFilter]
     time_grains: List[str]
     dimensions: List[str]
     resource_type: NodeType = field(metadata={"restrict": [NodeType.Metric]})
+    timestamp: Optional[str] = None
     window: Optional[MetricTime] = None
     model: Optional[str] = None
     model_unique_id: Optional[str] = None

core/dbt/contracts/graph/unparsed.py

@@ -484,9 +484,9 @@ class UnparsedMetric(dbtClassMixin, Replaceable):
     name: str
     label: str
     calculation_method: str
-    timestamp: str
     expression: str
     description: str = ""
+    timestamp: Optional[str] = None
     time_grains: List[str] = field(default_factory=list)
     dimensions: List[str] = field(default_factory=list)
     window: Optional[MetricTime] = None
@@ -518,6 +518,16 @@ def validate(cls, data):
                     f"The metric name '{data['name']}' is invalid.  It {', '.join(e for e in errors)}"
                 )
 
+        if data.get("timestamp") is None and data.get("time_grains") is not None:
+            raise ValidationError(
+                f"The metric '{data['name']} has time_grains defined but is missing a timestamp dimension."
+            )
+
+        if data.get("timestamp") is None and data.get("window") is not None:
+            raise ValidationError(
+                f"The metric '{data['name']} has a window defined but is missing a timestamp dimension."
+            )
+
         if data.get("model") is None and data.get("calculation_method") != "derived":
             raise ValidationError("Non-derived metrics require a 'model' property")
 

tests/functional/metrics/fixtures.py

@@ -642,3 +642,58 @@
     meta:
         my_meta: 'testing'
 """
+
+metric_without_timestamp_or_timegrains_yml = """
+version: 2
+
+metrics:
+  - name: number_of_people
+    label: "Number of people"
+    description: Total count of people
+    model: "ref('people')"
+    calculation_method: count
+    expression: "*"
+    dimensions:
+      - favorite_color
+      - loves_dbt
+    meta:
+        my_meta: 'testing'
+"""
+
+invalid_metric_without_timestamp_with_time_grains_yml = """
+version: 2
+
+metrics:
+  - name: number_of_people
+    label: "Number of people"
+    description: Total count of people
+    model: "ref('people')"
+    time_grains: [day, week, month]
+    calculation_method: count
+    expression: "*"
+    dimensions:
+      - favorite_color
+      - loves_dbt
+    meta:
+        my_meta: 'testing'
+"""
+
+invalid_metric_without_timestamp_with_window_yml = """
+version: 2
+
+metrics:
+  - name: number_of_people
+    label: "Number of people"
+    description: Total count of people
+    model: "ref('people')"
+    window:
+      count: 14
+      period: day
+    calculation_method: count
+    expression: "*"
+    dimensions:
+      - favorite_color
+      - loves_dbt
+    meta:
+        my_meta: 'testing'
+"""

tests/functional/metrics/test_metrics.py

@@ -3,6 +3,7 @@
 from dbt.tests.util import run_dbt, get_manifest
 from dbt.exceptions import ParsingException
 
+
 from tests.functional.metrics.fixtures import (
     mock_purchase_data_csv,
     models_people_sql,
@@ -18,6 +19,9 @@
     invalid_derived_metric_contains_model_yml,
     derived_metric_yml,
     derived_metric_old_attr_names_yml,
+    metric_without_timestamp_or_timegrains_yml,
+    invalid_metric_without_timestamp_with_time_grains_yml,
+    invalid_metric_without_timestamp_with_window_yml
 )
 
 
@@ -46,6 +50,33 @@ def test_simple_metric(
         assert metric_ids == expected_metric_ids
 
 
+class TestSimpleMetricsNoTimestamp:
+    @pytest.fixture(scope="class")
+    def models(self):
+        return {
+            "people_metrics.yml": metric_without_timestamp_or_timegrains_yml,
+            "people.sql": models_people_sql,
+        }
+
+    def test_simple_metric_no_timestamp(
+        self,
+        project,
+    ):
+        # initial run
+        results = run_dbt(["run"])
+        assert len(results) == 1
+        manifest = get_manifest(project.project_root)
+        metric_ids = list(manifest.metrics.keys())
+        expected_metric_ids = [
+            "metric.test.number_of_people",
+        ]
+        assert metric_ids == expected_metric_ids
+
+        # make sure the 'expression' metric depends on the two upstream metrics
+        metric_test = manifest.metrics["metric.test.number_of_people"]
+        assert metric_test.timestamp is None
+
+
 class TestInvalidRefMetrics:
     @pytest.fixture(scope="class")
     def models(self):
@@ -253,3 +284,41 @@ def models(self):
             "derived_metric.yml": derived_metric_old_attr_names_yml,
             "downstream_model.sql": downstream_model_sql,
         }
+
+
+class TestInvalidTimestampTimeGrainsMetrics:
+    @pytest.fixture(scope="class")
+    def models(self):
+        return {
+            "people_metrics.yml": invalid_metric_without_timestamp_with_time_grains_yml,
+            "people.sql": models_people_sql,
+        }
+
+    # Tests that we get a ParsingException with an invalid metric definition.
+    # This metric definition is missing timestamp but HAS a time_grains property
+    def test_simple_metric(
+        self,
+        project,
+    ):
+        # initial run
+        with pytest.raises(ParsingException):
+            run_dbt(["run"])
+
+
+class TestInvalidTimestampWindowMetrics:
+    @pytest.fixture(scope="class")
+    def models(self):
+        return {
+            "people_metrics.yml": invalid_metric_without_timestamp_with_window_yml,
+            "people.sql": models_people_sql,
+        }
+
+    # Tests that we get a ParsingException with an invalid metric definition.
+    # This metric definition is missing timestamp but HAS a window property
+    def test_simple_metric(
+        self,
+        project,
+    ):
+        # initial run
+        with pytest.raises(ParsingException):
+            run_dbt(["run"])