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

Altering Window Metric Attribute To Match Freshness Tests #5793

Merged
merged 7 commits into from
Sep 9, 2022
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
8 changes: 8 additions & 0 deletions .changes/unreleased/Under the Hood-20220908-180731.yaml
Original file line number Diff line number Diff line change
@@ -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"
3 changes: 2 additions & 1 deletion core/dbt/contracts/graph/parsed.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
ExposureType,
MaturityType,
MetricFilter,
MetricTime,
)
from dbt.contracts.util import Replaceable, AdditionalPropertiesMixin
from dbt.exceptions import warn_or_error
Expand Down Expand Up @@ -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
Expand Down
21 changes: 20 additions & 1 deletion core/dbt/contracts/graph/unparsed.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down
40 changes: 40 additions & 0 deletions schemas/dbt/manifest/v7.json
Original file line number Diff line number Diff line change
Expand Up @@ -6343,6 +6343,12 @@
"$ref": "#/definitions/MetricFilter"
}
},
"window": {
"type": "array",
"items": {
"$ref": "#/definitions/MetricTime"
}
},
"time_grains": {
"type": "array",
"items": {
Expand Down Expand Up @@ -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#",
Expand Down
17 changes: 12 additions & 5 deletions test/unit/test_contracts_graph_unparsed.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -697,7 +697,11 @@ def get_ok_dict(self):
"operator": "=",
}
],
'window': '14 days',
'window': {
"count": 14,
"period": "day"
}
,
'tags': [],
'meta': {
'is_okr': True
Expand All @@ -715,8 +719,8 @@ def get_ok_derived_dict(self):
'timestamp': 'signup_date',
'dimensions': [],
'filters': [],
'window': '',
'tags': [],
'window': {},
'meta': {
'is_okr': True
},
Expand All @@ -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()
Expand All @@ -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()
Expand Down
4 changes: 2 additions & 2 deletions test/unit/test_graph_selector_methods.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -380,7 +380,7 @@ def make_metric(pkg, name, path=None):
value=True,
operator="=",
)],
window='',
window=MetricTime(),
meta={'is_okr': True},
tags=['okrs'],
)
Expand Down
5 changes: 3 additions & 2 deletions test/unit/test_manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@
ExposureType,
ExposureOwner,
MaturityType,
MetricFilter
MetricFilter,
MetricTime
)

from dbt.contracts.graph.compiled import CompiledModelNode
Expand Down Expand Up @@ -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']],
Expand Down
8 changes: 4 additions & 4 deletions tests/functional/metrics/test_metric_deferral.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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]
Expand Down
4 changes: 3 additions & 1 deletion tests/functional/metrics/test_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down