-
Notifications
You must be signed in to change notification settings - Fork 175
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
update for generalized constraint warnings #551
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 dbt-snowflake contributing guide. |
5d2aaef
to
2718430
Compare
|
||
@classmethod | ||
def render_column_constraint(cls, constraint: ColumnLevelConstraint) -> str: | ||
if constraint.type == ConstraintType.check: |
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.
Maybe this is over-engineering it, but can we use CONSTRAINT_SUPPORT
? So this:
if self.CONSTRAINT_SUPPORT[constraint.type] == ConstraintSupport.NON_SUPPORTED:
return ""
return super().render_column_constraint(constraint)
For that matter, can we just put this in dbt-core
? Then we wouldn't need to override this method at all.
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'm not exactly sure what you mean by override – it seems that they're removing these methods entirely right?
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.
We did, and we also did what Mike suggested, but in the base adapter, so that everyone can enjoy.
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 don't read so gud
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.
Apparently if you stare at code long enough, you forget what red/green and left/right mean.
return super().render_column_constraint(constraint) | ||
|
||
@classmethod | ||
def render_model_constraint(cls, constraint: ModelLevelConstraint) -> Optional[str]: |
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.
Same as above.
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.
Assuming I'm reading my comment correctly and that we are in fact good to go
|
||
@classmethod | ||
def render_column_constraint(cls, constraint: ColumnLevelConstraint) -> str: | ||
if constraint.type == ConstraintType.check: |
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.
We did, and we also did what Mike suggested, but in the base adapter, so that everyone can enjoy.
resolves #492
Description
Uses centralized warnings from dbt-core for constraints.
Checklist
changie new
to create a changelog entry