-
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
make schemas available to on-run-end hooks (#908) #1028
Conversation
@@ -117,6 +120,7 @@ def compile_node(self, node, manifest): | |||
|
|||
context = dbt.context.runtime.generate( |
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.
couldn't the runtime context generate the list of schemas from the list of nodes? is there a reason to do this in on-run-end hooks based on the list of results, rather than here from the list of nodes?
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.
If you run with models selected, this way we only get the actually executed models.
8333a88
to
30b6868
Compare
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.
One comment for minor additional functionality, but this otherwise LGTM!
r.node.schema for r in results | ||
if not any((r.errored, r.failed, r.skipped)) | ||
)) | ||
cls.safe_run_hooks(config, adapter, manifest, RunHookType.End, |
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.
would it be crazy to just add the results
into extra_content
here too? I imagine it would be useful for users to iterate over the results and do something intelligent with errors, for example. Any drawbacks to that idea?
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 think it would be crazy at all, I wrote it as a dict for pretty much that reason. I'll add it in.
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.
haha, cool, let's do it :)
Implements #908. In on-run-end hooks (and only in on-run-end hooks!) the value
schemas
is now injected into the context. It's a list of unique schemas that dbt successfully executed models/seeds into.The general model here is adding an argument to
compile_node()
: a dict of "extra things" to add to the context, and plumbing it through from the ModelRunner when running on-run-end hooks. This injects the value into both the runtime context and the compilation context, though it's the latter that the test case actually cares about.