Support managed jdbc io (MySQL) #36045
Conversation
|
Assigning reviewers: R: @damccorm for label python. Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
shunping
force-pushed
the
managed-jdbcio-2
branch
from
d15e5b8 to
8b05bf6
Compare
shunping
force-pushed
the
managed-jdbcio-2
branch
from
8b05bf6 to
3121b02
Compare
ahmedabu98
left a comment
ahmedabu98
left a comment
There was a problem hiding this comment.
Overall looks good, just left a few comments
| String jdbcType = configuration.getJdbcType(); | ||
| if (jdbcType != null && !jdbcType.isEmpty() && !jdbcType.equals(jdbcType())) { | ||
| throw new IllegalArgumentException( | ||
| String.format("Wrong JDBC type. Expected '%s' but got '%s'", jdbcType(), jdbcType)); | ||
| } |
There was a problem hiding this comment.
Wondering if we should just be ignoring the jdbcType parameter. It seems irrelevant if users specifically choose MySQL using SchemaTransform/Managed API
There was a problem hiding this comment.
I have changed the exception to a warning message and overwritten the wrong type with the correct one.
| LOG.warn( | ||
| "The fetchSize option is ignored. It is required to set useCursorFetch=true" | ||
| + " in the JDBC URL when using fetchSize for MySQL"); |
There was a problem hiding this comment.
Consider failing instead of just logging a warning?
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #36045 +/- ##
==========================================
Coverage 54.91% 54.91%
Complexity 1617 1617
==========================================
Files 1057 1057
Lines 164001 164293 +292
Branches 1165 1165
==========================================
+ Hits 90061 90226 +165
- Misses 71787 71914 +127
Partials 2153 2153
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
Managed JDBCIO (Part 2) - MySQL
Similar to #36034