Behavior change for mf timespine without yaml configuration #10857
Conversation
|
Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #10857 +/- ##
==========================================
- Coverage 89.15% 89.07% -0.09%
==========================================
Files 183 183
Lines 23529 23550 +21
==========================================
- Hits 20977 20976 -1
- Misses 2552 2574 +22
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
The parts of the code that I'm familiar with look good to me! Not sure about the protos changes, etc so will need core eyes on those.
In the PR description, I would clarify that the schema has not changed again in case your core reviewer thinks there is a schema change involved in this PR - this is referencing a schema change that has already been released to users.
DevonFulcher
force-pushed
the
Behavior_change_for_mf_timespine_without_yaml_configuration
branch
from
e8f65d3 to
6da0bc6
Compare
core/dbt/contracts/project.py
Outdated
| @@ -344,6 +344,7 @@ class ProjectFlags(ExtensibleDbtClassMixin): | |||
| skip_nodes_if_on_run_start_fails: bool = False | |||
| state_modified_compare_more_unrendered_values: bool = False | |||
| state_modified_compare_vars: bool = False | |||
| allow_mf_time_spines_without_yaml_configuration: bool = False | |||
There was a problem hiding this comment.
What do you think about the naming, require_yaml_configuration_for_mf_time_spines: False instead?
It would be a more conventional nam and better follow our guides: https://github.com/dbt-labs/dbt-core/blob/main/docs/guides/behavior-change-flags.md#rules-for-introducing-a-new-flag
| @@ -1,5 +1,5 @@ | |||
| # Events Module | |||
| The Events module is responsible for communicating internal dbt structures into a consumable interface. Because the "event" classes are based entirely on protobuf definitions, the interface is really clearly defined, whether or not protobufs are used to consume it. We use Betterproto for compiling the protobuf message definitions into Python classes. | |||
| The Events module is responsible for communicating internal dbt structures into a consumable interface. Because the "event" classes are based entirely on protobuf definitions, the interface is really clearly defined, whether or not protobufs are used to consume it. We use protoc for compiling the protobuf message definitions into Python classes. | |||
There was a problem hiding this comment.
thank you for these updates <3
| ): | ||
| deprecations.warn( | ||
| "mf-timespine-without-yaml-configuration", | ||
| ) |
There was a problem hiding this comment.
Presumably this warning would not be terribly noisy -- (at most) one per invocation? Let me know if I'm off!
There was a problem hiding this comment.
Yes, this is validation for the semantic manifest which there is only one of per project. I can't think of a way that this would fire more than once per invocation.
|
Could you please create/link a corresponding issue in docs.getdbt.com so the docs team can work on documenting the new behaviour once this is merged? |
MichelleArk
deleted the
Behavior_change_for_mf_timespine_without_yaml_configuration
branch
Linear issue #SL-2935
Github issue #10959
docs: dbt-labs/docs.getdbt.com#6387
Problem
The configuration for the MetricFlow timespine has changed in a previous release. This deprecation warning was previously surfaced in DSI, but that is non-standard for dbt-core usage.
Solution
This PR implements a behavior change to inform users of the deprecation of the old time spine config.
Tests:
dbt parseonjaffle-sl-templateand saw no warningsjaffle-sl-templateto have ametricflow_time_spine.sqlmodel and saw the deprecation warning.dbt_project.yamlinjaffle-sl-templateand saw no warning fordbt parse:Checklist