-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
removed deprication for materialization-return and replaced it with a… #3893
Conversation
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 good to me. I can't think of anything you're missing.
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! Left one small comment that you can choose to take or leave.
Thanks also for splitting up the various deprecations into sub-tasks. Feeling much more organized :)
Let's hold off on merging this into develop
until we cut v0.21.0-rc1.
self.run_dbt(['run'], expect_pass=False) | ||
|
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.
If you want, you can check the results for the explicit error message you're expecting:
results = self.run_dbt(['run'], expect_pass=False)
assert 'did not explicitly return a list of relations' in results[0].message
I wouldn't say that's required here, though it could be a nice way to ensure we're providing users with a helpful error message, and not just failing silently.
382c52a
to
237048c
Compare
…/dbt into 2798_enact_deprecations # Conflicts: # test/integration/012_deprecation_tests/test_deprecations.py
@@ -1,3 +1,13 @@ | |||
## dbt 1.0.0 (Release TBD) |
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.
🎉
resolves #3896
Description
Enact deprecation for
materialization-return
. Removed the deprecation warning and replaced it with an exception.Looking for feedback to see if there's anything I'm overlooking (other than updating the CHANGELOG.md). I tested locally and ran all tests.
Checklist
CHANGELOG.md
and added information about my change to the "dbt next" section.