-
Notifications
You must be signed in to change notification settings - Fork 975
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
add mf timespine behavior change #6408
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hiya @mirnawong1
Thanks for creating this. I've approved this for you and committed one change for readability. I've also made two non-blocking suggestions for readability.
Kind Regards
Natalie
Co-authored-by: nataliefiann <[email protected]>
Co-authored-by: nataliefiann <[email protected]>
|
||
Set the flag to `True` to require YAML configuration for MetricFlow time spine file for dbt Cloud Versionless or dbt Core 1.9 and later. In previous versions (dbt Core 1.8 and earlier), the MetricFlow time spine configuration was stored in a `metricflow_time_spine.sql` file. | ||
|
||
When the flag is set to `True`, dbt will raise a deprecation warning if it detects a MetricFlow time spine configured in a SQL file. When the flag is set to `False`, dbt will continue to support the SQL file configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for updating these docs, but this is the other way around. See my note in the PR description here
I added this to dbt_project.yaml in jaffle-sl-template and saw no warning for dbt parse:
flags:
require_yaml_configuration_for_mf_time_spines: True
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh right sorry, i missed the tests. thanks for clarifying @DevonFulcher ! so the flag allows users to continue to support the SQL file configuration. I'll update!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated it to this @DevonFulcher :
MetricFlow time spine YAML
The require_yaml_configuration_for_mf_time_spines
flag is set to False
by default.
Set the flag to True
to require YAML configuration for MetricFlow time spine file for dbt Cloud Versionless or dbt Core 1.9 and later. In previous versions (dbt Core 1.8 and earlier), the MetricFlow time spine configuration was stored in a metricflow_time_spine.sql
file.
When the flag is set to True
, dbt will continue to support the SQL file configuration. When the flag is set to False
, dbt will raise a deprecation warning if it detects a MetricFlow time spine configured in a SQL file.
The MetricFlow YAML file should be named models/_models.yml
and should be located in the models
directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! So, both configurations are accepted for now, regardless of the value of require_yaml_configuration_for_mf_time_spines
, so this line isn't totally accurate:
Set the flag to True to require YAML configuration for MetricFlow time spine file for dbt Cloud Versionless or dbt Core 1.9 and later.
I think we can just delete that sentence since you describe the True/False behavior later in this section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good flag, removed it now @DevonFulcher !
|
||
When the flag is set to `True`, dbt will continue to support the SQL file configuration. When the flag is set to `False`, dbt will raise a deprecation warning if it detects a MetricFlow time spine configured in a SQL file. | ||
|
||
The MetricFlow YAML file should be named `models/_models.yml` and should be located in the `models` directory. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the model can be named anything as long as it has the time_spine:
field. Maybe we just link to the v1.9 docs here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! I just left a small comment.
this pr adds the new behavior change flag
require_yaml_configuration_for_mf_time_spines
to the behavior change doc.it also adds a MF section and links the flags to the appropriate sections.
Resolves #6387 and used dbt-labs/dbt-core#10857 as guidance.
🚀 Deployment available! Here are the direct links to the updated files: