[Backport to 1.7.latest] Fix assorted source freshness edgecases so check is run or actionable information (#9825) #9826
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… information (#9825) * Ensure BaseRunner handles nodes without `build_path` Some nodes, like SourceDefinition nodes, don't have a `build_path` property. This is problematic because we take in nodes with no type checking, and assume they have properties sometimes, like `build_path`. This was just the case in BaseRunner's `_handle_generic_exception` and `_handle_interal_exception` methods. Thus to stop dbt from crashing when trying to handle an exception related to a node without a `build_path`, we added an private method to the BaseRunner class for safely trying to get `build_path`. * Use keyword arguments when instantiating `Note` events in freshness.py Previously we were passing arguments during the `Note` event instantiations in freshness.py as positional arguments. This would cause not the desired `Note` event to be emitted, but instead get the message ``` [Note] Don't use positional arguments when constructing logging events ``` which was our fault, not the users'. Additionally, we were passing the level for the event in the `Note` instantiation when we needed to be passing it to the `fire_event` method. * Raise error when `loaded_at_field` is `None` and metadata check isn't possible Previously if a source freshness check didn't have a `loaded_at_field` and metadata source freshness wasn't supported by the adapter, then we'd log a warning message and let the source freshness check continue. This was problematic because the source freshness check couldn't actually continue and the process would raise an error in the form ``` type object argument after ** must be a mapping, not NoneType ``` because the `freshness` variable was never getting set. This error wasn't particularly helpful for any person running into it. So instead of letting that error happen we now deliberately raise an error with helpful information. * Add test which ensures bad source freshness checks raise appropriate error This test directly tests that when a source freshness check doesn't have a `loaded_at_field` and the adapter in use doesn't support metadata checks, then the appropriate error message gets raised. That is, it directly tests the change made in a162d53. This test indirectly tests the changes in both 7ec2f82 and 7b0ff31 as the appropriate error can only be raised because we've fixed other upstream issues via those commits. * Add changelog entry for source freshness edgecase fixes
Moving things to dbt.artifacts started to happen AFTER the release of dbt-core 1.7, thus 1.7 has no concept of dbt.articacts. The changes brought in via the previous commit, were cherry-picked from main (1.8.0 beta) and thus reference dbt.artifacts. This caused everything to blow up, because dbt.artifacts doesn't exist. Here we've updated the dbt.artifacts imports to their pre-dbt.artifacts paths.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 1.7.latest #9826 +/- ##
===========================================
Coverage 86.62% 86.63%
===========================================
Files 179 179
Lines 26671 26672 +1
===========================================
+ Hits 23105 23108 +3
+ Misses 3566 3564 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
graciegoheen
approved these changes
Mar 29, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Manual backport of a029661 from #9825