-
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
Quote config #742
Quote config #742
Conversation
…cs/dbt into quote-config
dbt/project.py
Outdated
'behavior for Snowflake will change in a future release, ' | ||
'so it is recommended that you define this explicitly. ' | ||
'\n\n' | ||
'For more information, see ADD LINK') |
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.
TODO
dbt/adapters/snowflake/relation.py
Outdated
@@ -11,7 +11,7 @@ class SnowflakeRelation(DefaultRelation): | |||
'quote_policy': { | |||
'database': False, | |||
'schema': False, | |||
'identifier': False, | |||
'identifier': True, |
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.
should we quote both schema and identifier here? Feels weird to do one and not the other
dbt/main.py
Outdated
@@ -175,6 +175,7 @@ def invoke_dbt(parsed): | |||
profile_to_load=parsed.profile, | |||
args=parsed | |||
) | |||
proj.log_warnings() |
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 should do this check on line 235. If --target
is used on the CLI, then this will refer to the wrong target. Probably not an issue for most, but i just got snagged locally
No description provided.