-
Notifications
You must be signed in to change notification settings - Fork 960
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
Model property deprecation_date #3579
Conversation
✅ Deploy Preview for docs-getdbt-com ready!
To edit notification comments on pull requests, go to your Netlify site 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.
This is looking good!
I added inline suggestions for using a full "aware" datetime rather than just a "naive" date.
See below for two more suggestions.
1. Model governance
It seems like we should update https://docs.getdbt.com/docs/collaborate/govern/model-versions to add a section and examples for deprecation_date
.
2. Supported formats
I haven't tried any of these formats yet, but I'd hope / expect that they all work:
canonical: 2001-12-15T02:59:43.1Z
iso8601: 2001-12-14t21:59:43.10-05:00
spaced: 2001-12-14 21:59:43.10 -5
date: 2002-12-14
In particular, I'm hoping that all RFC 3339 timestamp formats are supported (as well as the most common ISO 8601 formats).
See here for examples of different formats:
https://dbeatty10.github.io/rfc3339-iso8601/
models: | ||
- name: my_model | ||
description: deprecating in the future | ||
deprecation_date: 2999-01-01 |
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.
See datetime format comment above.
Agree with Doug -- we should add a new section to the docs on "Model Versions" about
|
Co-authored-by: Doug Beatty <44704949+dbeatty10@users.noreply.github.com>
@@ -31,6 +31,10 @@ exports.versions = [ | |||
] | |||
|
|||
exports.versionedPages = [ | |||
{ |
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.
trying to fix deploy preview build error
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.
fixed the deploy error and the preview looks fine. two questions, i wanted to flag is:
- do we want to version the model-properties code so
deprecation_date
appears if you're on 1.6 or higher? or are actively showing it so ppl can upgrade versions? - should we add
deprecation_date
in the https://docs.getdbt.com/docs/collaborate/govern/model-versions page?
What are you changing in this pull request and why?
Adding new model property
deprecation_date
Create new model property card
Add to existing table of properties
Checklist
Uncomment if you're publishing docs for a prerelease version of dbt (delete if not applicable):
Adding new pages (delete if not applicable):
website/sidebars.js
Link to Core repo PR
dbt-labs/dbt-core#7562