-
Notifications
You must be signed in to change notification settings - Fork 40
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
add saved queries and exports to latest #129
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.
All comments non-blocking except the fact that this reformats a huge swath of arrays which should hopefully be easy to fix!
"type", | ||
"type_params" | ||
], | ||
"required": ["name", "label", "type", "type_params"], |
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 assume that these are because your formatting rules are different to what we normally use? They should be able to look at the config in the .vscode settings dir (or whatever it is) to get the style guide
"type": "string" | ||
} | ||
}, | ||
"additionalProperties": 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.
I think this breaks autocomplete or something? So i lean towards being false and we keep up to date. Without evidence, it's a weak opinion weakly held.
"$ref": "#/$defs/array_of_strings" | ||
}, | ||
"where": { | ||
"$ref": "#/$defs/array_of_strings" |
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 wonder if it's worth checking with a regex match for {{ Dimension
for a lot of these, because it feels like a common mistake mode to just provide the name of the dim without the wrapper. (related)
closes #126
This PR adds support for SQEs for latest version of dbt. I used the docs here to guide implementation.
Two areas for discussion that i don't feel strongly about:
cache
as an available value in exports'export_as
field based on the docs saying "coming soon", but I am very willing to remove it until it's actually available .additionalProperties: True
for export configs assuming that there will be more configs over time. I can make this more restrictive if we wantsorry for the unintentional whitespace edits 😅