diff --git a/.changes/unreleased/Breaking Changes-20220906-154521.yaml b/.changes/unreleased/Breaking Changes-20220906-154521.yaml new file mode 100644 index 00000000000..7275adecd78 --- /dev/null +++ b/.changes/unreleased/Breaking Changes-20220906-154521.yaml @@ -0,0 +1,7 @@ +kind: Breaking Changes +body: Renaming Metric Spec Attributes +time: 2022-09-06T15:45:21.2769-05:00 +custom: + Author: callum-mcdata + Issue: "5774" + PR: "5775" diff --git a/core/dbt/contracts/graph/metrics.py b/core/dbt/contracts/graph/metrics.py index 3ab307bfe54..20222b4a32b 100644 --- a/core/dbt/contracts/graph/metrics.py +++ b/core/dbt/contracts/graph/metrics.py @@ -49,13 +49,17 @@ def parent_metrics_names(cls, metric_node, manifest): @classmethod def reverse_dag_parsing(cls, metric_node, manifest, metric_depth_count): - if metric_node.type == "expression": + if metric_node.calculation_method == "derived": yield {metric_node.name: metric_depth_count} metric_depth_count = metric_depth_count + 1 for parent_unique_id in metric_node.depends_on.nodes: node = manifest.metrics.get(parent_unique_id) - if node and node.resource_type == NodeType.Metric and node.type == "expression": + if ( + node + and node.resource_type == NodeType.Metric + and node.calculation_method == "derived" + ): yield from cls.reverse_dag_parsing(node, manifest, metric_depth_count) def full_metric_dependency(self): @@ -67,7 +71,7 @@ def base_metric_dependency(self): to_return = [] for metric in in_scope_metrics: - if metric.type != "expression" and metric.name not in to_return: + if metric.calculation_method != "derived" and metric.name not in to_return: to_return.append(metric.name) return to_return @@ -77,7 +81,7 @@ def derived_metric_dependency(self): to_return = [] for metric in in_scope_metrics: - if metric.type == "expression" and metric.name not in to_return: + if metric.calculation_method == "derived" and metric.name not in to_return: to_return.append(metric.name) return to_return diff --git a/core/dbt/contracts/graph/parsed.py b/core/dbt/contracts/graph/parsed.py index 1758d6f210a..0f0b5055c0b 100644 --- a/core/dbt/contracts/graph/parsed.py +++ b/core/dbt/contracts/graph/parsed.py @@ -806,8 +806,8 @@ class ParsedMetric(UnparsedBaseNode, HasUniqueID, HasFqn): name: str description: str label: str - type: str - sql: str + calculation_method: str + expression: str timestamp: Optional[str] filters: List[MetricFilter] time_grains: List[str] @@ -850,11 +850,11 @@ def same_description(self, old: "ParsedMetric") -> bool: def same_label(self, old: "ParsedMetric") -> bool: return self.label == old.label - def same_type(self, old: "ParsedMetric") -> bool: - return self.type == old.type + def same_calculation_method(self, old: "ParsedMetric") -> bool: + return self.calculation_method == old.calculation_method - def same_sql(self, old: "ParsedMetric") -> bool: - return self.sql == old.sql + def same_expression(self, old: "ParsedMetric") -> bool: + return self.expression == old.expression def same_timestamp(self, old: "ParsedMetric") -> bool: return self.timestamp == old.timestamp @@ -875,8 +875,8 @@ def same_contents(self, old: Optional["ParsedMetric"]) -> bool: and self.same_filters(old) and self.same_description(old) and self.same_label(old) - and self.same_type(old) - and self.same_sql(old) + and self.same_calculation_method(old) + and self.same_expression(old) and self.same_timestamp(old) and self.same_time_grains(old) and True diff --git a/core/dbt/contracts/graph/unparsed.py b/core/dbt/contracts/graph/unparsed.py index 07dcf67def1..2de0ac668ed 100644 --- a/core/dbt/contracts/graph/unparsed.py +++ b/core/dbt/contracts/graph/unparsed.py @@ -451,13 +451,13 @@ class UnparsedMetric(dbtClassMixin, Replaceable): # name: Identifier name: str label: str - type: str + calculation_method: str timestamp: str - model: Optional[str] = None description: str = "" - sql: Union[str, int] = "" + expression: Union[str, int] = "" time_grains: List[str] = field(default_factory=list) dimensions: List[str] = field(default_factory=list) + model: Optional[str] = None window: Optional[str] = None filters: List[MetricFilter] = field(default_factory=list) meta: Dict[str, Any] = field(default_factory=dict) @@ -471,9 +471,13 @@ def validate(cls, data): if "name" in data and " " in data["name"]: raise ParsingException(f"Metrics name '{data['name']}' cannot contain spaces") - # TODO: Expressions _cannot_ have `model` properties - if data.get("model") is None and data.get("type") != "expression": - raise ValidationError("Non-expression metrics require a 'model' property") + if data.get("calculation_method") == "expression": + raise ValidationError( + "The metric calculation method expression has been deprecated and renamed to derived. Please update" + ) + + if data.get("model") is None and data.get("calculation_method") != "derived": + raise ValidationError("Non-derived metrics require a 'model' property") - if data.get("model") is not None and data.get("type") == "expression": - raise ValidationError("Expression metrics cannot have a 'model' property") + if data.get("model") is not None and data.get("calculation_method") == "derived": + raise ValidationError("Derived metrics cannot have a 'model' property") diff --git a/core/dbt/parser/schema_renderer.py b/core/dbt/parser/schema_renderer.py index 71a7632636e..d2e5ce90b13 100644 --- a/core/dbt/parser/schema_renderer.py +++ b/core/dbt/parser/schema_renderer.py @@ -67,7 +67,7 @@ def should_render_keypath(self, keypath: Keypath) -> bool: elif self._is_norender_key(keypath[0:]): return False elif self.key == "metrics": - if keypath[0] == "sql": + if keypath[0] == "expression": return False elif self._is_norender_key(keypath[0:]): return False diff --git a/core/dbt/parser/schemas.py b/core/dbt/parser/schemas.py index 4fd081c55a8..94bc4c5fa8c 100644 --- a/core/dbt/parser/schemas.py +++ b/core/dbt/parser/schemas.py @@ -1046,8 +1046,8 @@ def parse_metric(self, unparsed: UnparsedMetric) -> ParsedMetric: name=unparsed.name, description=unparsed.description, label=unparsed.label, - type=unparsed.type, - sql=str(unparsed.sql), + calculation_method=unparsed.calculation_method, + expression=str(unparsed.expression), timestamp=unparsed.timestamp, dimensions=unparsed.dimensions, window=unparsed.window, @@ -1068,8 +1068,8 @@ def parse_metric(self, unparsed: UnparsedMetric) -> ParsedMetric: model_ref = "{{ " + parsed.model + " }}" get_rendered(model_ref, ctx, parsed) - parsed.sql = get_rendered( - parsed.sql, + parsed.expression = get_rendered( + parsed.expression, ctx, node=parsed, ) diff --git a/test/integration/068_partial_parsing_tests/test-files/env_var_metrics.yml b/test/integration/068_partial_parsing_tests/test-files/env_var_metrics.yml index 01bf32212a8..b8112fea010 100644 --- a/test/integration/068_partial_parsing_tests/test-files/env_var_metrics.yml +++ b/test/integration/068_partial_parsing_tests/test-files/env_var_metrics.yml @@ -6,8 +6,8 @@ metrics: name: number_of_people description: Total count of people label: "Number of people" - type: count - sql: "*" + calculation_method: count + expression: "*" timestamp: created_at time_grains: [day, week, month] dimensions: @@ -20,8 +20,8 @@ metrics: name: collective_tenure description: Total number of years of team experience label: "Collective tenure" - type: sum - sql: tenure + calculation_method: sum + expression: tenure timestamp: created_at time_grains: [day] filters: diff --git a/test/integration/068_partial_parsing_tests/test-files/my_metric.yml b/test/integration/068_partial_parsing_tests/test-files/my_metric.yml index d91de1607cc..521bc92290f 100644 --- a/test/integration/068_partial_parsing_tests/test-files/my_metric.yml +++ b/test/integration/068_partial_parsing_tests/test-files/my_metric.yml @@ -4,8 +4,8 @@ metrics: label: New Customers model: customers description: "The number of paid customers who are using the product" - type: count - sql: user_id + calculation_method: count + expression: user_id timestamp: signup_date time_grains: [day, week, month] dimensions: diff --git a/test/integration/068_partial_parsing_tests/test-files/people_metrics.yml b/test/integration/068_partial_parsing_tests/test-files/people_metrics.yml index 763bc0bcafb..99d31a4e632 100644 --- a/test/integration/068_partial_parsing_tests/test-files/people_metrics.yml +++ b/test/integration/068_partial_parsing_tests/test-files/people_metrics.yml @@ -6,8 +6,8 @@ metrics: name: number_of_people description: Total count of people label: "Number of people" - type: count - sql: "*" + calculation_method: count + expression: "*" timestamp: created_at time_grains: [day, week, month] dimensions: @@ -20,8 +20,8 @@ metrics: name: collective_tenure description: Total number of years of team experience label: "Collective tenure" - type: sum - sql: tenure + calculation_method: sum + expression: tenure timestamp: created_at time_grains: [day] filters: diff --git a/test/integration/068_partial_parsing_tests/test-files/people_metrics2.yml b/test/integration/068_partial_parsing_tests/test-files/people_metrics2.yml index 96b10a89510..5f826e66e85 100644 --- a/test/integration/068_partial_parsing_tests/test-files/people_metrics2.yml +++ b/test/integration/068_partial_parsing_tests/test-files/people_metrics2.yml @@ -6,8 +6,8 @@ metrics: name: number_of_people description: Total count of people label: "Number of people" - type: count - sql: "*" + calculation_method: count + expression: "*" timestamp: created_at time_grains: [day, week, month] dimensions: @@ -20,8 +20,8 @@ metrics: name: collective_tenure description: Total number of years of team experience label: "Collective tenure" - type: sum - sql: tenure + calculation_method: sum + expression: tenure timestamp: created_at time_grains: [day] filters: diff --git a/test/unit/test_contracts_graph_parsed.py b/test/unit/test_contracts_graph_parsed.py index 114a31ecff3..16faae24e7e 100644 --- a/test/unit/test_contracts_graph_parsed.py +++ b/test/unit/test_contracts_graph_parsed.py @@ -2318,8 +2318,8 @@ def basic_parsed_metric_dict(): 'name': 'new_customers', 'label': 'New Customers', 'model': 'ref("dim_customers")', - 'type': 'count', - 'sql': 'user_id', + 'calculation_method': 'count', + 'expression': 'user_id', 'timestamp': 'signup_date', 'time_grains': ['day', 'week', 'month'], 'dimensions': ['plan', 'country'], @@ -2355,7 +2355,7 @@ def basic_parsed_metric_dict(): def basic_parsed_metric_object(): return ParsedMetric( name='my_metric', - type='count', + calculation_method='count', fqn=['test', 'metrics', 'my_metric'], unique_id='metric.test.my_metric', package_name='test', diff --git a/test/unit/test_contracts_graph_unparsed.py b/test/unit/test_contracts_graph_unparsed.py index 4fa3aaaf719..c69235969ef 100644 --- a/test/unit/test_contracts_graph_unparsed.py +++ b/test/unit/test_contracts_graph_unparsed.py @@ -685,8 +685,8 @@ def get_ok_dict(self): 'label': 'New Customers', 'model': 'ref("dim_customers")', 'description': 'New customers', - 'type': 'count', - 'sql': 'user_id', + 'calculation_method': 'count', + 'expression': 'user_id', 'timestamp': 'signup_date', 'time_grains': ['day', 'week', 'month'], 'dimensions': ['plan', 'country'], @@ -704,15 +704,15 @@ def get_ok_dict(self): }, } - def get_ok_expression_dict(self): + def get_ok_derived_dict(self): return { 'name': 'arpc', 'label': 'revenue per customer', 'description': '', - 'type': 'expression', - 'sql': "{{ metric('revenue') }} / {{ metric('customers') }}", - 'timestamp': "signup_date", + 'calculation_method': 'derived', + 'expression': "{{ metric('revenue') }} / {{ metric('customers') }}", 'time_grains': ['day', 'week', 'month'], + 'timestamp': 'signup_date', 'dimensions': [], 'filters': [], 'window': '', @@ -728,8 +728,8 @@ def test_ok(self): label='New Customers', model='ref("dim_customers")', description="New customers", - type='count', - sql="user_id", + calculation_method='count', + expression="user_id", timestamp="signup_date", time_grains=['day', 'week', 'month'], dimensions=['plan', 'country'], @@ -746,34 +746,34 @@ def test_ok(self): pickle.loads(pickle.dumps(metric)) def test_ok_metric_no_model(self): - # Expression metrics do not have model properties + # Derived metrics do not have model properties metric = self.ContractType( name='arpc', label='revenue per customer', model=None, description="", - type='expression', - sql="{{ metric('revenue') }} / {{ metric('customers') }}", + calculation_method='derived', + expression="{{ metric('revenue') }} / {{ metric('customers') }}", timestamp="signup_date", time_grains=['day', 'week', 'month'], dimensions=[], window='', meta={'is_okr': True}, ) - dct = self.get_ok_expression_dict() + dct = self.get_ok_derived_dict() self.assert_symmetric(metric, dct) pickle.loads(pickle.dumps(metric)) - def test_bad_metric_no_type(self): + def test_bad_metric_no_calculation_method(self): tst = self.get_ok_dict() - del tst['type'] + del tst['calculation_method'] self.assert_fails_validation(tst) def test_bad_metric_no_model(self): tst = self.get_ok_dict() - # Metrics with type='expression' do not have model props + # Metrics with calculation_type='derived' do not have model props tst['model'] = None - tst['type'] = 'sum' + tst['calculation_method'] = 'sum' self.assert_fails_validation(tst) def test_bad_filter_missing_things(self): diff --git a/test/unit/test_graph_selector_methods.py b/test/unit/test_graph_selector_methods.py index e640a3146f6..000152a50d4 100644 --- a/test/unit/test_graph_selector_methods.py +++ b/test/unit/test_graph_selector_methods.py @@ -370,8 +370,8 @@ def make_metric(pkg, name, path=None): label='New Customers', model='ref("multi")', description="New customers", - type='count', - sql="user_id", + calculation_method='count', + expression="user_id", timestamp="signup_date", time_grains=['day', 'week', 'month'], dimensions=['plan', 'country'], diff --git a/test/unit/test_manifest.py b/test/unit/test_manifest.py index 83e36e78428..7dc582cb3a8 100644 --- a/test/unit/test_manifest.py +++ b/test/unit/test_manifest.py @@ -103,8 +103,8 @@ def setUp(self): label='New Customers', model='ref("multi")', description="New customers", - type='count', - sql="user_id", + calculation_method='count', + expression="user_id", timestamp="signup_date", time_grains=['day', 'week', 'month'], dimensions=['plan', 'country'], diff --git a/test/unit/test_yaml_renderer.py b/test/unit/test_yaml_renderer.py index e132248c7b5..57b841f7960 100644 --- a/test/unit/test_yaml_renderer.py +++ b/test/unit/test_yaml_renderer.py @@ -110,15 +110,15 @@ def test__metrics(self): dct = { "name": "my_source", "description": "{{ docs('my_doc') }}", - "sql": "select {{ var('my_var') }} from my_table", + "expression": "select {{ var('my_var') }} from my_table", "time_grains": "{{ my_time_grains }}", } - # We expect the sql and description will not be rendered, but + # We expect the expression and description will not be rendered, but # other fields will be expected = { "name": "my_source", "description": "{{ docs('my_doc') }}", - "sql": "select {{ var('my_var') }} from my_table", + "expression": "select {{ var('my_var') }} from my_table", "time_grains": "[day]" } dct = renderer.render_data(dct) diff --git a/tests/functional/metrics/test_metric_helper_functions.py b/tests/functional/metrics/test_metric_helper_functions.py index 6c60ea7a985..7a1c09821b5 100644 --- a/tests/functional/metrics/test_metric_helper_functions.py +++ b/tests/functional/metrics/test_metric_helper_functions.py @@ -12,8 +12,8 @@ label: "Number of people" description: Total count of people model: "ref('people')" - type: count - sql: "*" + calculation_method: count + expression: "*" timestamp: created_at time_grains: [day, week, month] dimensions: @@ -26,8 +26,8 @@ label: "Collective tenure" description: Total number of years of team experience model: "ref('people')" - type: sum - sql: tenure + calculation_method: sum + expression: tenure timestamp: created_at time_grains: [day, week, month] filters: @@ -38,16 +38,16 @@ - name: average_tenure label: "Average tenure" description: "The average tenure per person" - type: expression - sql: "{{metric('collective_tenure')}} / {{metric('number_of_people')}} " + calculation_method: derived + expression: "{{metric('collective_tenure')}} / {{metric('number_of_people')}} " timestamp: created_at time_grains: [day, week, month] - name: average_tenure_plus_one label: "Average tenure" description: "The average tenure per person" - type: expression - sql: "{{metric('average_tenure')}} + 1 " + calculation_method: derived + expression: "{{metric('average_tenure')}} + 1 " timestamp: created_at time_grains: [day, week, month] """ diff --git a/tests/functional/metrics/test_metrics.py b/tests/functional/metrics/test_metrics.py index c7fd831c0be..8e31293f207 100644 --- a/tests/functional/metrics/test_metrics.py +++ b/tests/functional/metrics/test_metrics.py @@ -15,8 +15,8 @@ label: "Number of people" description: Total count of people model: "ref('people')" - type: count - sql: "*" + calculation_method: count + expression: "*" timestamp: created_at time_grains: [day, week, month] dimensions: @@ -29,8 +29,8 @@ label: "Collective tenure" description: Total number of years of team experience model: "ref('people')" - type: sum - sql: tenure + calculation_method: sum + expression: tenure timestamp: created_at time_grains: [day] filters: @@ -42,8 +42,8 @@ label: "Collective window" description: Testing window model: "ref('people')" - type: sum - sql: tenure + calculation_method: sum + expression: tenure timestamp: created_at time_grains: [day] window: 14 days @@ -97,8 +97,8 @@ def test_simple_metric( label: "Number of people" description: Total count of people model: "ref(people)" - type: count - sql: "*" + calculation_method: count + expression: "*" timestamp: created_at time_grains: [day, week, month] dimensions: @@ -111,8 +111,8 @@ def test_simple_metric( label: "Collective tenure" description: Total number of years of team experience model: "ref(people)" - type: sum - sql: tenure + calculation_method: sum + expression: tenure timestamp: created_at time_grains: [day] filters: @@ -150,8 +150,8 @@ def test_simple_metric( - name: number_of_people label: "Number of people" description: Total count of people - type: count - sql: "*" + calculation_method: count + expression: "*" timestamp: created_at time_grains: [day, week, month] dimensions: @@ -163,8 +163,8 @@ def test_simple_metric( - name: collective_tenure label: "Collective tenure" description: Total number of years of team experience - type: sum - sql: tenure + calculation_method: sum + expression: tenure timestamp: created_at time_grains: [day] filters: @@ -203,8 +203,8 @@ def test_simple_metric( label: "Number of people" description: Total count of people model: "ref('people')" - type: count - sql: "*" + calculation_method: count + expression: "*" timestamp: created_at time_grains: [day, week, month] dimensions: @@ -217,8 +217,8 @@ def test_simple_metric( label: "Collective tenure" description: Total number of years of team experience model: "ref('people')" - type: sum - sql: tenure + calculation_method: sum + expression: tenure timestamp: created_at time_grains: [day] filters: @@ -272,8 +272,8 @@ def test_names_with_spaces(self, project): {% for m in some_metrics %} name: {{ m.name }} label: {{ m.label }} - type: {{ m.type }} - sql: {{ m.sql }} + calculation_method: {{ m.calculation_method }} + expression: {{ m.expression }} timestamp: {{ m.timestamp }} time_grains: {{ m.time_grains }} dimensions: {{ m.dimensions }} @@ -286,15 +286,15 @@ def test_names_with_spaces(self, project): select 1 as id """ -invalid_expression_metric__contains_model_yml = """ +invalid_derived_metric__contains_model_yml = """ version: 2 metrics: - name: count_orders label: Count orders model: ref('mock_purchase_data') - type: count - sql: "*" + calculation_method: count + expression: "*" timestamp: purchased_at time_grains: [day, week, month, quarter, year] @@ -305,8 +305,8 @@ def test_names_with_spaces(self, project): label: Total order revenue model: ref('mock_purchase_data') - type: sum - sql: "payment_total" + calculation_method: sum + expression: "payment_total" timestamp: purchased_at time_grains: [day, week, month, quarter, year] @@ -316,8 +316,8 @@ def test_names_with_spaces(self, project): - name: average_order_value label: Average Order Value - type: expression - sql: "{{metric('sum_order_revenue')}} / {{metric('count_orders')}} " + calculation_method: derived + expression: "{{metric('sum_order_revenue')}} / {{metric('count_orders')}} " model: ref('mock_purchase_data') timestamp: purchased_at time_grains: [day, week, month, quarter, year] @@ -327,28 +327,28 @@ def test_names_with_spaces(self, project): """ -class TestInvalidExpressionMetrics: +class TestInvalidDerivedMetrics: @pytest.fixture(scope="class") def models(self): return { - "expression_metric.yml": invalid_expression_metric__contains_model_yml, + "derived_metric.yml": invalid_derived_metric__contains_model_yml, "downstream_model.sql": downstream_model_sql, } - def test_invalid_expression_metrics(self, project): + def test_invalid_derived_metrics(self, project): with pytest.raises(ParsingException): run_dbt(["run"]) -expression_metric_yml = """ +derived_metric_yml = """ version: 2 metrics: - name: count_orders label: Count orders model: ref('mock_purchase_data') - type: count - sql: "*" + calculation_method: count + expression: "*" timestamp: purchased_at time_grains: [day, week, month, quarter, year] @@ -359,8 +359,8 @@ def test_invalid_expression_metrics(self, project): label: Total order revenue model: ref('mock_purchase_data') - type: sum - sql: "payment_total" + calculation_method: sum + expression: "payment_total" timestamp: purchased_at time_grains: [day, week, month, quarter, year] @@ -370,8 +370,8 @@ def test_invalid_expression_metrics(self, project): - name: average_order_value label: Average Order Value - type: expression - sql: "{{metric('sum_order_revenue')}} / {{metric('count_orders')}} " + calculation_method: derived + expression: "{{metric('sum_order_revenue')}} / {{metric('count_orders')}} " timestamp: purchased_at time_grains: [day, week, month, quarter, year] @@ -380,11 +380,11 @@ def test_invalid_expression_metrics(self, project): """ -class TestExpressionMetric: +class TestDerivedMetric: @pytest.fixture(scope="class") def models(self): return { - "expression_metric.yml": expression_metric_yml, + "derived_metric.yml": derived_metric_yml, "downstream_model.sql": downstream_model_sql, } @@ -397,7 +397,7 @@ def seeds(self): "mock_purchase_data.csv": mock_purchase_data_csv, } - def test_expression_metric( + def test_derived_metric( self, project, ): @@ -426,9 +426,9 @@ def test_expression_metric( assert sorted(downstream_model.config["metric_names"]) == metric_names # make sure the 'expression' metric depends on the two upstream metrics - expression_metric = manifest.metrics["metric.test.average_order_value"] - assert sorted(expression_metric.metrics) == [["count_orders"], ["sum_order_revenue"]] - assert sorted(expression_metric.depends_on.nodes) == [ + derived_metric = manifest.metrics["metric.test.average_order_value"] + assert sorted(derived_metric.metrics) == [["count_orders"], ["sum_order_revenue"]] + assert sorted(derived_metric.depends_on.nodes) == [ "metric.test.count_orders", "metric.test.sum_order_revenue", ] @@ -443,8 +443,8 @@ def test_expression_metric( for property in [ "name", "label", - "type", - "sql", + "calculation_method", + "expression", "timestamp", "time_grains", "dimensions",