-
Notifications
You must be signed in to change notification settings - Fork 58
Generate clearer exception for Avro schema translation failures #1021
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
base: master
Are you sure you want to change the base?
Conversation
| } catch (Exception e) { | ||
| throw new RuntimeException(String.format( | ||
| "Failed to build Avro schema for sqlType %d %s [%s, %s]", | ||
| columnType, typeName, columnClassName, columnTypeName | ||
| ), e); | ||
| } |
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 mind the idea. I think in the new version of dbeam though the exceptions won't be as bad, but wrapping the code and reporting which field had an issue could still be useful anyways. Maybe it would look cleaner with a new helper function such as convertField(), then in createAvroFields() in the loop, we call convertField() on each field and that is wrapped in try-catch block.
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.
nice idea! will do
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.
ugh, tried out the helper function and i think it was overall messier because there's like 5 variables we need to pass in per field, and extra 2 containing state from the outer loop. might be more trouble than it's worth :(
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.
ok fair enough, I'm fine with just wrapping the whole code block in try-catch then
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1021 +/- ##
============================================
+ Coverage 91.92% 91.96% +0.03%
Complexity 283 283
============================================
Files 27 27
Lines 1015 1020 +5
Branches 86 86
============================================
+ Hits 933 938 +5
Misses 54 54
Partials 28 28 🚀 New features to boost your workflow:
|
No description provided.