Skip to content

Commit

Permalink
[Backport 1.7.latest] Update parser to support conversion metrics #9173
Browse files Browse the repository at this point in the history
  (#9255)

* Move minimum DSI version to 0.4.2

We're backporting a feature "conversion metrics" to 1.7. Conversion
metrics don't exist in DSI < 0.4.2 which is problematic if we allow
for those versions. This ensures that those who are on a version of
1.7 that supports conversion metrics will also have the requisit version
of DSI.

* added ConversionTypeParams classes

* updated parser for ConversionTypeParams

* added step to populate input_measure for conversion metrics

* added tests

* added changelog

* Regenerate v11 manifest jsonschema to include conversion metrics definition

* Regenerate v11 manifest test artifact for testing version compatability

---------

Co-authored-by: Will Deng <[email protected]>
  • Loading branch information
QMalcolm and WilliamDee authored Dec 11, 2023
1 parent 0ef8638 commit d65179b
Show file tree
Hide file tree
Showing 12 changed files with 701 additions and 29 deletions.
7 changes: 7 additions & 0 deletions .changes/unreleased/Features-20231206-181458.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
kind: Features
body: Adds support for parsing conversion metric related properties for the semantic
layer.
time: 2023-12-06T18:14:58.688221-05:00
custom:
Author: WilliamDee
Issue: "9203"
2 changes: 1 addition & 1 deletion core/dbt/contracts/graph/metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@


DERIVED_METRICS = [MetricType.DERIVED, MetricType.RATIO]
BASE_METRICS = [MetricType.SIMPLE, MetricType.CUMULATIVE]
BASE_METRICS = [MetricType.SIMPLE, MetricType.CUMULATIVE, MetricType.CONVERSION]


class MetricReference(object):
Expand Down
18 changes: 17 additions & 1 deletion core/dbt/contracts/graph/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
SourceFileMetadata,
)
from dbt.contracts.graph.unparsed import (
ConstantPropertyInput,
Docs,
ExposureType,
ExternalTable,
Expand Down Expand Up @@ -59,7 +60,11 @@
TimeDimensionReference,
)
from dbt_semantic_interfaces.references import MetricReference as DSIMetricReference
from dbt_semantic_interfaces.type_enums import MetricType, TimeGranularity
from dbt_semantic_interfaces.type_enums import (
ConversionCalculationType,
MetricType,
TimeGranularity,
)

from .model_config import (
NodeConfig,
Expand Down Expand Up @@ -1435,6 +1440,16 @@ def post_aggregation_reference(self) -> DSIMetricReference:
return DSIMetricReference(element_name=self.alias or self.name)


@dataclass
class ConversionTypeParams(dbtClassMixin):
base_measure: MetricInputMeasure
conversion_measure: MetricInputMeasure
entity: str
calculation: ConversionCalculationType = ConversionCalculationType.CONVERSION_RATE
window: Optional[MetricTimeWindow] = None
constant_properties: Optional[List[ConstantPropertyInput]] = None


@dataclass
class MetricTypeParams(dbtClassMixin):
measure: Optional[MetricInputMeasure] = None
Expand All @@ -1445,6 +1460,7 @@ class MetricTypeParams(dbtClassMixin):
window: Optional[MetricTimeWindow] = None
grain_to_date: Optional[TimeGranularity] = None
metrics: Optional[List[MetricInput]] = None
conversion_type_params: Optional[ConversionTypeParams] = None


@dataclass
Expand Down
20 changes: 20 additions & 0 deletions core/dbt/contracts/graph/unparsed.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from dbt.exceptions import CompilationError, ParsingError, DbtInternalError

from dbt.dataclass_schema import dbtClassMixin, StrEnum, ExtensibleDbtClassMixin, ValidationError
from dbt_semantic_interfaces.type_enums import ConversionCalculationType

from dataclasses import dataclass, field
from datetime import timedelta
Expand Down Expand Up @@ -597,6 +598,24 @@ class UnparsedMetricInput(dbtClassMixin):
offset_to_grain: Optional[str] = None # str is really a TimeGranularity Enum


@dataclass
class ConstantPropertyInput(dbtClassMixin):
base_property: str
conversion_property: str


@dataclass
class UnparsedConversionTypeParams(dbtClassMixin):
base_measure: Union[UnparsedMetricInputMeasure, str]
conversion_measure: Union[UnparsedMetricInputMeasure, str]
entity: str
calculation: str = (
ConversionCalculationType.CONVERSION_RATE.value
) # ConversionCalculationType Enum
window: Optional[str] = None
constant_properties: Optional[List[ConstantPropertyInput]] = None


@dataclass
class UnparsedMetricTypeParams(dbtClassMixin):
measure: Optional[Union[UnparsedMetricInputMeasure, str]] = None
Expand All @@ -606,6 +625,7 @@ class UnparsedMetricTypeParams(dbtClassMixin):
window: Optional[str] = None
grain_to_date: Optional[str] = None # str is really a TimeGranularity Enum
metrics: Optional[List[Union[UnparsedMetricInput, str]]] = None
conversion_type_params: Optional[UnparsedConversionTypeParams] = None


@dataclass
Expand Down
57 changes: 40 additions & 17 deletions core/dbt/parser/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -1527,43 +1527,66 @@ def _process_refs(
node.depends_on.add_node(target_model_id)


def _process_metric_node(
def _process_metric_depends_on(
manifest: Manifest,
current_project: str,
metric: Metric,
) -> None:
"""Sets a metric's `input_measures` and `depends_on` properties"""

# This ensures that if this metrics input_measures have already been set
# we skip the work. This could happen either due to recursion or if multiple
# metrics derive from another given metric.
# NOTE: This does not protect against infinite loops
if len(metric.type_params.input_measures) > 0:
return
"""For a given metric, set the `depends_on` property"""

if metric.type is MetricType.SIMPLE or metric.type is MetricType.CUMULATIVE:
assert (
metric.type_params.measure is not None
), f"{metric} should have a measure defined, but it does not."
metric.type_params.input_measures.append(metric.type_params.measure)
assert len(metric.type_params.input_measures) > 0
for input_measure in metric.type_params.input_measures:
target_semantic_model = manifest.resolve_semantic_model_for_measure(
target_measure_name=metric.type_params.measure.name,
target_measure_name=input_measure.name,
current_project=current_project,
node_package=metric.package_name,
)
if target_semantic_model is None:
raise dbt.exceptions.ParsingError(
f"A semantic model having a measure `{metric.type_params.measure.name}` does not exist but was referenced.",
f"A semantic model having a measure `{input_measure.name}` does not exist but was referenced.",
node=metric,
)
if target_semantic_model.config.enabled is False:
raise dbt.exceptions.ParsingError(
f"The measure `{metric.type_params.measure.name}` is referenced on disabled semantic model `{target_semantic_model.name}`.",
f"The measure `{input_measure.name}` is referenced on disabled semantic model `{target_semantic_model.name}`.",
node=metric,
)

metric.depends_on.add_node(target_semantic_model.unique_id)


def _process_metric_node(
manifest: Manifest,
current_project: str,
metric: Metric,
) -> None:
"""Sets a metric's `input_measures` and `depends_on` properties"""

# This ensures that if this metrics input_measures have already been set
# we skip the work. This could happen either due to recursion or if multiple
# metrics derive from another given metric.
# NOTE: This does not protect against infinite loops
if len(metric.type_params.input_measures) > 0:
return

if metric.type is MetricType.SIMPLE or metric.type is MetricType.CUMULATIVE:
assert (
metric.type_params.measure is not None
), f"{metric} should have a measure defined, but it does not."
metric.type_params.input_measures.append(metric.type_params.measure)
_process_metric_depends_on(
manifest=manifest, current_project=current_project, metric=metric
)
elif metric.type is MetricType.CONVERSION:
conversion_type_params = metric.type_params.conversion_type_params
assert (
conversion_type_params
), f"{metric.name} is a conversion metric and must have conversion_type_params defined."
metric.type_params.input_measures.append(conversion_type_params.base_measure)
metric.type_params.input_measures.append(conversion_type_params.conversion_measure)
_process_metric_depends_on(
manifest=manifest, current_project=current_project, metric=metric
)
elif metric.type is MetricType.DERIVED or metric.type is MetricType.RATIO:
input_metrics = metric.input_metrics
if metric.type is MetricType.RATIO:
Expand Down
30 changes: 24 additions & 6 deletions core/dbt/parser/schema_yaml_readers.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
UnparsedQueryParams,
UnparsedSavedQuery,
UnparsedSemanticModel,
UnparsedConversionTypeParams,
)
from dbt.contracts.graph.model_config import SavedQueryConfig
from dbt.contracts.graph.nodes import (
Expand All @@ -29,6 +30,7 @@
MetricTypeParams,
SemanticModel,
SavedQuery,
ConversionTypeParams,
)
from dbt.contracts.graph.saved_queries import Export, ExportConfig, QueryParams
from dbt.contracts.graph.semantic_layer_common import WhereFilter, WhereFilterIntersection
Expand All @@ -52,6 +54,7 @@
from dbt.dataclass_schema import ValidationError
from dbt_semantic_interfaces.type_enums import (
AggregationType,
ConversionCalculationType,
DimensionType,
EntityType,
MetricType,
Expand Down Expand Up @@ -226,7 +229,7 @@ def _get_time_window(
self.yaml.path,
"window",
{"window": unparsed_window},
f"Invalid window ({unparsed_window}) in cumulative metric. Should be of the form `<count> <granularity>`, "
f"Invalid window ({unparsed_window}) in cumulative/conversion metric. Should be of the form `<count> <granularity>`, "
"e.g., `28 days`",
)

Expand All @@ -240,7 +243,7 @@ def _get_time_window(
self.yaml.path,
"window",
{"window": unparsed_window},
f"Invalid time granularity {granularity} in cumulative metric window string: ({unparsed_window})",
f"Invalid time granularity {granularity} in cumulative/conversion metric window string: ({unparsed_window})",
)

count = parts[0]
Expand All @@ -249,7 +252,7 @@ def _get_time_window(
self.yaml.path,
"window",
{"window": unparsed_window},
f"Invalid count ({count}) in cumulative metric window string: ({unparsed_window})",
f"Invalid count ({count}) in cumulative/conversion metric window string: ({unparsed_window})",
)

return MetricTimeWindow(
Expand Down Expand Up @@ -295,6 +298,20 @@ def _get_metric_inputs(

return metric_inputs

def _get_optional_conversion_type_params(
self, unparsed: Optional[UnparsedConversionTypeParams]
) -> Optional[ConversionTypeParams]:
if unparsed is None:
return None
return ConversionTypeParams(
base_measure=self._get_input_measure(unparsed.base_measure),
conversion_measure=self._get_input_measure(unparsed.conversion_measure),
entity=unparsed.entity,
calculation=ConversionCalculationType(unparsed.calculation),
window=self._get_time_window(unparsed.window),
constant_properties=unparsed.constant_properties,
)

def _get_metric_type_params(self, type_params: UnparsedMetricTypeParams) -> MetricTypeParams:
grain_to_date: Optional[TimeGranularity] = None
if type_params.grain_to_date is not None:
Expand All @@ -308,9 +325,10 @@ def _get_metric_type_params(self, type_params: UnparsedMetricTypeParams) -> Metr
window=self._get_time_window(type_params.window),
grain_to_date=grain_to_date,
metrics=self._get_metric_inputs(type_params.metrics),
# TODO This is a compiled list of measure/numerator/denominator as
# well as the `input_measures` of included metrics. We're planning
# on doing this as part of CT-2707
conversion_type_params=self._get_optional_conversion_type_params(
type_params.conversion_type_params
)
# input measures are calculated via metric processing post parsing
# input_measures=?,
)

Expand Down
2 changes: 1 addition & 1 deletion core/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
"dbt-extractor~=0.5.0",
"minimal-snowplow-tracker~=0.0.2",
# DSI is under active development, so we're pinning to specific dev versions for now.
"dbt-semantic-interfaces~=0.4.0",
"dbt-semantic-interfaces~=0.4.2",
# ----
# Expect compatibility with all new versions of these packages, so lower bounds only.
"jsonschema>=3.0",
Expand Down
Loading

0 comments on commit d65179b

Please sign in to comment.