-
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
Rename "constraints_enabled" to "contract" #7002
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. |
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.
Changes here look good! There is some boilerplate in each adapter implementation that references config.get('constraints_enabled')
(dbt-snowflake example), which will need to be updated as well. I can make corresponding issues in the adapter repos 👍
We should close the corresponding adapter issues prior to merging this to prevent breaking tests in adapters, since they inherit from functional tests modified in this PR. |
We can't close the adapter issues prior to merging this because the "contract" config wouldn't actually exist in core until after this is merged. The has to merge first, then all of the adapter issues. |
Actually, I have a question about this now that I think about it. We're "promoting" the "contract" attribute to the model level. Shouldn't we be accessing it via the model, like "model.contract", and not looking at the config? This is one of those fields where the config is just used to shuttle the value in node level... |
So actually the config.get calls should still work in the adapter repos since it doesn't require that the config attribute actually work. But if we did model.contract they might fail. |
@gshank thanks for evolving contracts by actually having the config called |
resolves #6748
Description
After more discussion and considering the scope of this setting, we decided to rename "constraints_enabled" to "contract".
Checklist
changie new
to create a changelog entry