Skip to content
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.

Sign up for GitHub

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

Jump to bottom

ct-2198: Unify constraints and check_constraints fields #7130

Merged
merged 22 commits into from
Mar 22, 2023
Merged

ct-2198: Unify constraints and check_constraints fields #7130

merged 22 commits into from
Mar 22, 2023

Conversation

peterallenwebb
Copy link
Contributor

@peterallenwebb peterallenwebb commented Mar 6, 2023 •
edited
Loading

resolves #7066

Description

Checklist

@cla-bot cla-bot bot added the cla:yes label Mar 6, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Mar 6, 2023

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.

gshank
gshank requested changes Mar 7, 2023
View reviewed changes
Copy link
Contributor

@gshank gshank left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks pretty good, except that the field I put in the wrong place should be moved :)

core/dbt/contracts/graph/unparsed.py Show resolved Hide resolved
@peterallenwebb peterallenwebb changed the title ct-2198: clean up some type names and uses ct-2198: Unify constraints and check_constraints fields Mar 14, 2023
gshank and others added 10 commits March 14, 2023 10:51
@gshank @peterallenwebb
Make mypy version consistently 0.981 (#7134)
8b352de
@gshank @peterallenwebb
CT 1808 diff based partial parsing (#6873)
95020ec
@emmyoop @peterallenwebb
model contracts on models materialized as views (#7120)
ad470e7 
* first pass

* rename tests

* fix failing test

* changelog

* fix functional test

* Update core/dbt/parser/base.py

* Update core/dbt/parser/schemas.py
@stu-k @peterallenwebb
Create method for env var deprecation (#7086)
15c327a
@colin-rogers-dbt @peterallenwebb
update to allow adapters to change model name resolution in py models (…
d00eb96 
…#7115)

* update to allow adapters to change model name resolution in py models

* add changie

* fix newline adds

* move quoting into macro

* use single quotes
@leo-schick @jtcohen6 @peterallenwebb
add env DBT_PROJECT_DIR support #6078 (#6659)
5945b60 
Co-authored-by: Jeremy Cohen <[email protected]>
@FishtownBuildBot @peterallenwebb
Add new index.html and changelog yaml files from dbt-docs (#7141)
27eeb5c
@dave-connors-3 @peterallenwebb
Make version configs optional (#7060)
53f1471
@aranke @FishtownBuildBot @peterallenwebb
[CT-1584] New top level commands: interactive compile (#7008)
996cfa8 
Co-authored-by: Github Build Bot <[email protected]>
@peterallenwebb
Merge remote-tracking branch 'origin/main' into paw/ct-2198-unify-con…
4e46095 
…straints
MichelleArk
MichelleArk approved these changes Mar 15, 2023
View reviewed changes
Copy link
Contributor

@MichelleArk MichelleArk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue #7066 also describes adding a ModelLevelConstraint dataclass that is available as a model property. However, we won't implement any usage of ModelLevelConstraint until #6754, so I'd be fine shipping this work as-is and introducing ModelLevelConstraint as part of #6754 instead.

peterallenwebb
peterallenwebb commented Mar 15, 2023
View reviewed changes
core/dbt/contracts/graph/unparsed.py Show resolved Hide resolved
core/dbt/contracts/graph/unparsed.py Show resolved Hide resolved
core/dbt/contracts/graph/unparsed.py Show resolved Hide resolved
core/dbt/contracts/graph/unparsed.py Show resolved Hide resolved
core/dbt/contracts/graph/unparsed.py Show resolved Hide resolved
core/dbt/contracts/graph/unparsed.py Show resolved Hide resolved
core/dbt/contracts/graph/unparsed.py Show resolved Hide resolved
core/dbt/parser/schemas.py Show resolved Hide resolved
core/dbt/parser/schemas.py Show resolved Hide resolved
core/dbt/contracts/graph/unparsed.py Show resolved Hide resolved
@peterallenwebb peterallenwebb marked this pull request as ready for review March 15, 2023 21:36
@peterallenwebb peterallenwebb requested review from a team as code owners March 15, 2023 21:36
gshank
gshank previously requested changes Mar 16, 2023
View reviewed changes
Copy link
Contributor

@gshank gshank left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this handles invalid column level constraint types.

core/dbt/parser/schemas.py Show resolved Hide resolved
MichelleArk
MichelleArk reviewed Mar 21, 2023
View reviewed changes
core/dbt/adapters/base/impl.py Outdated
@@ -1262,6 +1265,39 @@ def get_incremental_strategy_macro(self, model_context, strategy: str):
# This returns a callable macro
return model_context[macro_name]

@classmethod
def _parse_constraint(cls, raw_constraint: Dict[str, Any]) -> ColumnLevelConstraint:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

naming nit: _parse_column_constraint

@MichelleArk MichelleArk self-requested a review March 21, 2023 23:27
MichelleArk
MichelleArk approved these changes Mar 21, 2023
View reviewed changes
Copy link
Contributor

@MichelleArk MichelleArk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! A couple comments, nothing blocking.

@peterallenwebb peterallenwebb dismissed gshank’s stale review March 22, 2023 15:22

Already addressed requested changes.

@peterallenwebb peterallenwebb merged commit 73ff497 into main Mar 22, 2023
@peterallenwebb peterallenwebb deleted the paw/ct-2198-unify-constraints branch March 22, 2023 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jtcohen6 jtcohen6 jtcohen6 left review comments

@gshank gshank gshank approved these changes

@MichelleArk MichelleArk MichelleArk approved these changes

@mikealfare mikealfare mikealfare approved these changes

@VersusFacit VersusFacit Awaiting requested review from VersusFacit VersusFacit was automatically assigned from dbt-labs/adapters

@emmyoop emmyoop Awaiting requested review from emmyoop emmyoop is a code owner automatically assigned from dbt-labs/core-team

Assignees
No one assigned
Labels
cla:yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[CT-2198] Unify constraints and constraints_check configs