Add More Typing to the dbt.task Module #10622
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 contributing guide. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10622 +/- ##
=======================================
Coverage 88.91% 88.92%
=======================================
Files 180 180
Lines 22760 22776 +16
=======================================
+ Hits 20237 20253 +16
Misses 2523 2523
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| @@ -547,6 +547,8 @@ def compile_node( | |||
| the node's raw_code into compiled_code, and then calls the | |||
| recursive method to "prepend" the ctes. | |||
| """ | |||
| # REVIEW: UnitTestDefinition shouldn't be possible here because of the | |||
| # type of node, and it is likewise an invalid return type. | |||
There was a problem hiding this comment.
I don't actually remember why this is here. We went around a couple of times on different ways of handling unit tests, so my guess is that this might have come from an earlier iteration, before we switched to making a separate unit test manifest. My recommendation is to remove it and see if any tests fail.
| @@ -28,18 +29,19 @@ def handle_exception(self, e, ctx): | |||
| exc=str(e), exc_info=traceback.format_exc(), node_info=self.node.node_info | |||
| ) | |||
| ) | |||
| # REVIEW: This code is invalid and will always throw. | |||
There was a problem hiding this comment.
My guess here is that sometimes we were getting DbtRuntimeErrors without a node, and some of the message creation wouldn't work, but that subsequent refactoring has made it non-functional? It seems like a pretty bad thing if it always throws. Does anything break if it's removed?
Add type annotations, with only a few minor code changes.
Checklist