Altering Window Metric Attribute To Match Freshness Tests (#5793)

* changing window spec * more updates * adding to v7 json? * chenyu rules * updating for formatting * updating metric deferral test
dbt-labs · Sep 9, 2022 · b26280d · b26280d
1 parent cfece2c
commit b26280d
Show file tree

Hide file tree

Showing 9 changed files with 94 additions and 16 deletions.
diff --git a/.changes/unreleased/Under the Hood-20220908-180731.yaml b/.changes/unreleased/Under the Hood-20220908-180731.yaml
@@ -0,0 +1,8 @@
+kind: Under the Hood
+body: Reworking the way we define the window attribute of metrics to match freshness
+  tests
+time: 2022-09-08T18:07:31.532608-05:00
+custom:
+  Author: callum-mcdata
+  Issue: "5722"
+  PR: "5793"
diff --git a/core/dbt/contracts/graph/parsed.py b/core/dbt/contracts/graph/parsed.py
@@ -37,6 +37,7 @@
     ExposureType,
     MaturityType,
     MetricFilter,
+    MetricTime,
 )
 from dbt.contracts.util import Replaceable, AdditionalPropertiesMixin
 from dbt.exceptions import warn_or_error
@@ -812,7 +813,7 @@ class ParsedMetric(UnparsedBaseNode, HasUniqueID, HasFqn):
     filters: List[MetricFilter]
     time_grains: List[str]
     dimensions: List[str]
-    window: Optional[str]
+    window: Optional[MetricTime]
     model: Optional[str] = None
     model_unique_id: Optional[str] = None
     resource_type: NodeType = NodeType.Metric

diff --git a/core/dbt/contracts/graph/unparsed.py b/core/dbt/contracts/graph/unparsed.py
@@ -444,6 +444,25 @@ class MetricFilter(dbtClassMixin, Replaceable):
     value: str
 
 
+class MetricTimePeriod(StrEnum):
+    day = "day"
+    week = "week"
+    month = "month"
+    year = "year"
+
+    def plural(self) -> str:
+        return str(self) + "s"
+
+
+@dataclass
+class MetricTime(dbtClassMixin, Mergeable):
+    count: Optional[int] = None
+    period: Optional[MetricTimePeriod] = None
+
+    def __bool__(self):
+        return self.count is not None and self.period is not None
+
+
 @dataclass
 class UnparsedMetric(dbtClassMixin, Replaceable):
     # TODO : verify that this disallows metric names with spaces
@@ -457,8 +476,8 @@ class UnparsedMetric(dbtClassMixin, Replaceable):
     expression: Union[str, int] = ""
     time_grains: List[str] = field(default_factory=list)
     dimensions: List[str] = field(default_factory=list)
+    window: Optional[MetricTime] = None
     model: Optional[str] = None
-    window: Optional[str] = None
     filters: List[MetricFilter] = field(default_factory=list)
     meta: Dict[str, Any] = field(default_factory=dict)
     tags: List[str] = field(default_factory=list)

diff --git a/schemas/dbt/manifest/v7.json b/schemas/dbt/manifest/v7.json
@@ -6343,6 +6343,12 @@
             "$ref": "#/definitions/MetricFilter"
           }
         },
+        "window": {
+          "type": "array",
+          "items": {
+            "$ref": "#/definitions/MetricTime"
+          }
+        },
         "time_grains": {
           "type": "array",
           "items": {
@@ -6470,6 +6476,40 @@
       },
       "additionalProperties": false,
       "description": "MetricFilter(field: str, operator: str, value: str)"
+    },
+    "MetricTime": {
+      "type": "object",
+      "required": [],
+      "properties": {
+        "count": {
+          "oneOf": [
+            {
+              "type": "integer"
+            },
+            {
+              "type": "null"
+            }
+          ]
+        },
+        "period": {
+          "oneOf": [
+            {
+              "type": "string",
+              "enum": [
+                "day",
+                "week",
+                "month",
+                "year"
+              ]
+            },
+            {
+              "type": "null"
+            }
+          ]
+        }
+      },
+      "additionalProperties": false,
+      "description": "MetricTime(count: Optional[int] = None, period: Optional[dbt.contracts.graph.unparsed.MetricTimePeriod] = None)"
     }
   },
   "$schema": "http://json-schema.org/draft-07/schema#",

diff --git a/test/unit/test_contracts_graph_unparsed.py b/test/unit/test_contracts_graph_unparsed.py
@@ -7,7 +7,7 @@
     FreshnessThreshold, Quoting, UnparsedSourceDefinition,
     UnparsedSourceTableDefinition, UnparsedDocumentationFile, UnparsedColumn,
     UnparsedNodeUpdate, Docs, UnparsedExposure, MaturityType, ExposureOwner,
-    ExposureType, UnparsedMetric, MetricFilter
+    ExposureType, UnparsedMetric, MetricFilter, MetricTime, MetricTimePeriod
 )
 from dbt.contracts.results import FreshnessStatus
 from dbt.node_types import NodeType
@@ -697,7 +697,11 @@ def get_ok_dict(self):
                     "operator": "=",
                 }
             ],
-            'window': '14 days',
+            'window': {
+                    "count": 14,
+                    "period": "day"
+                }
+            ,
             'tags': [],
             'meta': {
                 'is_okr': True
@@ -715,8 +719,8 @@ def get_ok_derived_dict(self):
             'timestamp': 'signup_date',
             'dimensions': [],
             'filters': [],
-            'window': '',
             'tags': [],
+            'window': {},
             'meta': {
                 'is_okr': True
             },
@@ -738,7 +742,10 @@ def test_ok(self):
                 value='True',
                 operator="=",
             )],
-            window="14 days",
+            window=MetricTime(
+                count=14,
+                period=MetricTimePeriod.day
+            ),
             meta={'is_okr': True},
         )
         dct = self.get_ok_dict()
@@ -756,8 +763,8 @@ def test_ok_metric_no_model(self):
             expression="{{ metric('revenue') }} / {{ metric('customers') }}",
             timestamp="signup_date",
             time_grains=['day', 'week', 'month'],
+            window=MetricTime(),
             dimensions=[],
-            window='',
             meta={'is_okr': True},
         )
         dct = self.get_ok_derived_dict()

diff --git a/test/unit/test_graph_selector_methods.py b/test/unit/test_graph_selector_methods.py
@@ -23,7 +23,7 @@
     ColumnInfo,
 )
 from dbt.contracts.graph.manifest import Manifest
-from dbt.contracts.graph.unparsed import ExposureType, ExposureOwner, MetricFilter
+from dbt.contracts.graph.unparsed import ExposureType, ExposureOwner, MetricFilter,MetricTime
 from dbt.contracts.state import PreviousState
 from dbt.node_types import NodeType
 from dbt.graph.selector_methods import (
@@ -380,7 +380,7 @@ def make_metric(pkg, name, path=None):
            value=True,
            operator="=",
         )],
-        window='',
+        window=MetricTime(),
         meta={'is_okr': True},
         tags=['okrs'],
     )

diff --git a/test/unit/test_manifest.py b/test/unit/test_manifest.py
@@ -29,7 +29,8 @@
     ExposureType,
     ExposureOwner,
     MaturityType,
-    MetricFilter
+    MetricFilter,
+    MetricTime
 )
 
 from dbt.contracts.graph.compiled import CompiledModelNode
@@ -115,7 +116,7 @@ def setUp(self):
                 )],
                 meta={'is_okr': True},
                 tags=['okrs'],
-                window='',
+                window=MetricTime(),
                 resource_type=NodeType.Metric,
                 depends_on=DependsOn(nodes=['model.root.multi']),
                 refs=[['multi']],

diff --git a/tests/functional/metrics/test_metric_deferral.py b/tests/functional/metrics/test_metric_deferral.py
@@ -11,8 +11,8 @@
     label: Some Metric
     model: ref('model_a')
 
-    type: count
-    sql: id
+    calculation_method: count
+    expression: id
 
     timestamp: ts
     time_grains: [day]
@@ -26,8 +26,8 @@
     label: Some Metric
     model: ref('model_a')
 
-    type: count
-    sql: user_id
+    calculation_method: count
+    expression: user_id
 
     timestamp: ts
     time_grains: [day]

diff --git a/tests/functional/metrics/test_metrics.py b/tests/functional/metrics/test_metrics.py
@@ -46,7 +46,9 @@
     expression: tenure
     timestamp: created_at
     time_grains: [day]
-    window: 14 days
+    window:
+      count: 14
+      period: day
     filters:
       - field: loves_dbt
         operator: 'is'