Support for redshift 821 #10448
Merged
Support for redshift 821 #10448
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
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #10448 +/- ##
==========================================
+ Coverage 88.76% 88.85% +0.09%
==========================================
Files 180 180
Lines 22564 22573 +9
==========================================
+ Hits 20029 20058 +29
+ Misses 2535 2515 -20
Flags with carried forward coverage won't be shown. Click here to find out more.
|
MichelleArk
reviewed
Jul 17, 2024
MichelleArk
reviewed
Jul 17, 2024
MichelleArk
reviewed
Jul 17, 2024
MichelleArk
reviewed
Jul 17, 2024
| if non_none_row_index is None: | ||
| fire_event( | ||
| SystemStdErr( | ||
| bmsg="Unit Test fixtures benefit from having at least one row free of Null values to ensure consistent column types. Failure to meet this recommendation can result in type mismatch errors between unit test source models and `expected` fixtures." |
There was a problem hiding this comment.
this is very clear from my perspective reading this diff but I'm worried it's somewhat verbose for an end-user reading this. What about something more straightforward like:
"Unit test fixutre <fixture-name> in <unit-test-name> does not have any row free of null values, which may cause type mismatch errors during unit test execution."
There was a problem hiding this comment.
I'll take this suggestion for the user facing exception to be raised in redshift
MichelleArk
approved these changes
Jul 22, 2024
4 tasks
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.
take 2 on #10366
Problem
Merge of the original code with an exception for a case the team thought to be unlikely to exist, did in fact exist in one of our downstream analytics projects.
I conferred with the team and we have landed on instead logging a warning and letting the code move ahead after making an effort to float non-
Nonevalue columns to the first row inserted from anexceptedfixture.Solution
Borrowing the existing
dbt_commonevent to avoid adding yet another message type for a one-off error. Also, we will propose floating this logic to dbt adapters at some point, so we don't want to bake in too much logic right now.Checklist