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

[Backport 1.7.latest] Update parser to support conversion metrics #9173 #9255

Merged
merged 8 commits into from
Dec 11, 2023
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
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 @@
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 (

Check warning on line 1582 in core/dbt/parser/manifest.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/parser/manifest.py#L1581-L1582

Added lines #L1581 - L1582 were not covered by tests
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(

Check warning on line 1587 in core/dbt/parser/manifest.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/parser/manifest.py#L1585-L1587

Added lines #L1585 - L1587 were not covered by tests
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 @@
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 @@
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 @@
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 @@

return metric_inputs

def _get_optional_conversion_type_params(
self, unparsed: Optional[UnparsedConversionTypeParams]
) -> Optional[ConversionTypeParams]:
if unparsed is None:
return None
return ConversionTypeParams(

Check warning on line 306 in core/dbt/parser/schema_yaml_readers.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/parser/schema_yaml_readers.py#L306

Added line #L306 was not covered by tests
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 @@
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