-
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
[Feature] Return agate_table
in dbt
run-operation
result
#10957
base: main
Are you sure you want to change the base?
Conversation
Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA. In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR. CLA has not been signed by users: @acjh |
@cla-bot check |
The cla-bot has been summoned, and re-checked this pull request! |
@@ -49,6 +49,9 @@ def _run_unsafe(self, package_name, macro_name) -> "agate.Table": | |||
macro_name, project=package_name, kwargs=macro_kwargs, macro_resolver=self.manifest | |||
) | |||
|
|||
if isinstance(res, str): | |||
return None |
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.
Note: When there is no explicit return
function call in the macro — like for the select_something
test — res
is a str
of newline characters and spaces, at least as observed with the Postgres and SQLite adapters.
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.
A more robust check is not isinstance(res, agate.Table)
but there appears to be deliberate avoidance of import agate
. Let me know if that is preferred.
Resolves #10956
Problem
dbtRunner
does not propagate the return value ofdbt
run-operation
.Solution
Propagate the return value of
dbt
run-operation
in the result asagate_table
:run
method ofRunOperationTask
, capture the return value of_run_unsafe()
in a variableexecute_macro_result
and pass it when instantiatingRunResult
withagate_table=execute_macro_result
.agate_table
field inRunResultOutput
, and propagateagate_result
fromRunResult
toRunResultOutput
in the class methodRunResultsArtifact.from_execution_results
.Sample usage:
Also:
process_run_result
to a class method ofRunResultsArtifact
to allow overrides if desired.Checklist
or tests are not required or relevant for this PR.or this PR has already received feedback and approval from Product or DX.