-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
fix: MySQL et al. super calls #23971
fix: MySQL et al. super calls #23971
Conversation
Codecov Report
@@ Coverage Diff @@
## master #23971 +/- ##
=======================================
Coverage 68.19% 68.19%
=======================================
Files 1941 1941
Lines 75242 75217 -25
Branches 8157 8146 -11
=======================================
- Hits 51308 51294 -14
+ Misses 21851 21845 -6
+ Partials 2083 2078 -5
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
LGTM
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.
LGTM
(cherry picked from commit 2af76fc)
(cherry picked from commit 2af76fc)
SUMMARY
When invoking
super
we shouldn't explicitly be specifying theMySQLEngineSpec
for thetype
andobject_or_type
arguments as there's no guarantee that theMySQLEngineSpec
class is the class which gets instantiated, i.e., this could serve as a base class for another connector such as a custom StarRocks connector.The issue with the current implementation is if you provide logic of the following,
when
adjust_engine_params
is invoked it's using theMySQLEnginSpec.enforce_uri_query_params
et al. class level parameters. Simply invokingsuper
without parameters resolves this issue.I grokked the entire engine specs and made sure that all
super
calls were of the formsuper()
which mean updating the Ocient connector as well.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
CI.
ADDITIONAL INFORMATION