Skip to content

Commit

Permalink
Support dbt-semantic-interfaces 0.3.0 (#8820)
Browse files Browse the repository at this point in the history
* changie doc for DSI 0.3.0 upgrade

* Gracefully handle v10 metric filters

* Fix iteration over metrics in `upgrade_v10_metric_filters`

* Update previous manifest version test fixtures to have more expressive metrics

* Regenerate the test v10 manifest artifact using the more expressive metrics from 904cc1e

To do this I cherry-picked 904cc1e onto my local 1.6.latest branch,
had the test regenerate the test v10 manifest artifact, and then over
wrote the test v10 manifest artifact on this branch (cherry-picking it
across the branches didn't work, had to copy paste :grimmace:)

* Regenerate test v11 manifest artifact using the fixture changes in 904cc1e

* Update `upgrade_v10_metric_filters` to handled disabled metrics

Regenerating the v10 and v11 test manifest artifacts uncovered an
issue wherein we weren't handling disabled metrics that need to
get upgraded. This commit fixes that. Additionally, the
`upgrade_v10_metric_filters` was getting a bit unwieldy, so I broke
extracted the abstracted sub functions.

* Fix `test_backwards_compatible_versions` test

When we regenerated the v10 test manifest artifact, it started having
the `metricflow_time_sine` model, and it didn't previously. This caused
`test_backwards_compatible_versions` to start failing because it was
no longer identified as having modified state for v10. The test has
been altered accordingly
  • Loading branch information
QMalcolm authored Oct 11, 2023
1 parent af0cbcb commit 35f214d
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 6 deletions.
6 changes: 6 additions & 0 deletions .changes/unreleased/Dependencies-20231011-022252.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Dependencies
body: Upgrade dbt-semantic-interfaces dep to 0.3.0
time: 2023-10-11T02:22:52.231648-07:00
custom:
Author: QMalcolm
PR: "8819"
62 changes: 62 additions & 0 deletions core/dbt/contracts/graph/manifest_upgrade.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,72 @@ def drop_v9_and_prior_metrics(manifest: dict) -> None:
manifest["disabled"] = filtered_disabled_entries


def _convert_dct_with_filter(v10_dct_with_opt_filter):
"""Upgrage the filter object from v10 to v11.
v10 filters from a serialized manifest looked like:
{..., 'filter': {'where_sql_template': '<filter_value>'}}
whereas v11 filters look like:
{..., 'filter': {'where_filters': [{'where_sql_template': '<filter_value>'}, ...]}}
"""
if v10_dct_with_opt_filter is not None and v10_dct_with_opt_filter.get("filter") is not None:
v10_dct_with_opt_filter["filter"] = {"where_filters": [v10_dct_with_opt_filter["filter"]]}


def _convert_metric(v10_metric_dict):
"""Upgrades a v10 metric object to a v11 metric object.
Specifcally the following properties change
1. metric.filter
2. metric.type_params.measure.filter
3. metric.type_params.input_measures[x].filter
4. metric.type_params.numerator.filter
5. metric.type_params.denominator.filter
6. metric.type_params.metrics[x].filter"
"""

# handles top level metric filter
_convert_dct_with_filter(v10_metric_dict)

type_params = v10_metric_dict.get("type_params")
if type_params is not None:
_convert_dct_with_filter(type_params.get("measure"))
_convert_dct_with_filter(type_params.get("numerator"))
_convert_dct_with_filter(type_params.get("denominator"))

# handles metric.type_params.input_measures[x].filter
input_measures = type_params.get("input_measures")
if input_measures is not None:
for input_measure in input_measures:
_convert_dct_with_filter(input_measure)

# handles metric.type_params.metrics[x].filter
metrics = type_params.get("metrics")
if metrics is not None:
for metric in metrics:
_convert_dct_with_filter(metric)


def upgrade_v10_metric_filters(manifest: dict):
"""Handles metric filters changes from v10 to v11."""

metrics = manifest.get("metrics", {})
for metric in metrics.values():
_convert_metric(metric)

disabled_nodes = manifest.get("disabled", {})
for unique_id, nodes in disabled_nodes.items():
if unique_id.split(".")[0] == "metric":
for node in nodes:
_convert_metric(node)


def upgrade_manifest_json(manifest: dict, manifest_schema_version: int) -> dict:
# this should remain 9 while the check in `upgrade_schema_version` may change
if manifest_schema_version <= 9:
drop_v9_and_prior_metrics(manifest=manifest)
elif manifest_schema_version == 10:
upgrade_v10_metric_filters(manifest=manifest)

for node_content in manifest.get("nodes", {}).values():
upgrade_node_content(node_content)
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/artifacts/data/state/v10/manifest.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion tests/functional/artifacts/data/state/v11/manifest.json

Large diffs are not rendered by default.

38 changes: 34 additions & 4 deletions tests/functional/artifacts/test_previous_version_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,18 +161,46 @@
agg_time_dimension: created_at
metrics:
- name: my_metric
label: Count records
- name: blue_customers_post_2010
label: Blue Customers since 2010
type: simple
filter: "{{ TimeDimension('id__created_at', 'day') }} > '2010-01-01'"
type_params:
measure:
name: customers
filter: "{{ Dimension('id__favorite_color') }} = 'blue'"
- name: customers
label: Customers Metric
type: simple
type_params:
measure: customers
- name: disabled_metric
label: Count records
config:
enabled: False
filter: "{{ Dimension('id__favorite_color') }} = 'blue'"
type: simple
type_params:
measure: customers
- name: ratio_of_blue_customers_to_red_customers
label: Very Important Customer Color Ratio
type: ratio
type_params:
numerator:
name: customers
filter: "{{ Dimension('id__favorite_color')}} = 'blue'"
denominator:
name: customers
filter: "{{ Dimension('id__favorite_color')}} = 'red'"
- name: doubled_blue_customers
type: derived
label: Inflated blue customer numbers
type_params:
expr: 'customers * 2'
metrics:
- name: customers
filter: "{{ Dimension('id__favorite_color')}} = 'blue'"
sources:
- name: my_source
Expand Down Expand Up @@ -290,7 +318,7 @@ def test_project(self, project):
assert len(manifest.nodes) == 8
assert len(manifest.sources) == 1
assert len(manifest.exposures) == 1
assert len(manifest.metrics) == 1
assert len(manifest.metrics) == 4
# disabled model, snapshot, seed, singular test, generic test, analysis, source, exposure, metric
assert len(manifest.disabled) == 9
assert "macro.test.do_nothing" in manifest.macros
Expand Down Expand Up @@ -348,8 +376,10 @@ def test_compare_state_current(self, project):

def test_backwards_compatible_versions(self, project):
# manifest schema version 4 and greater should always be forward compatible
for schema_version in range(4, self.CURRENT_EXPECTED_MANIFEST_VERSION):
for schema_version in range(4, 10):
self.compare_previous_state(project, schema_version, True, 1)
for schema_version in range(10, self.CURRENT_EXPECTED_MANIFEST_VERSION):
self.compare_previous_state(project, schema_version, True, 0)

def test_nonbackwards_compatible_versions(self, project):
# schema versions 1, 2, 3 are all not forward compatible
Expand Down

0 comments on commit 35f214d

Please sign in to comment.