-
Notifications
You must be signed in to change notification settings - Fork 15.2k
fix: use risingwave as the sqlalchemy_uri_placeholder prefix for RisingWave engine #33764
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
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've completed my review and didn't find any issues.
Files scanned
File Path | Reviewed |
---|---|
superset/db_engine_specs/risingwave.py | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Check out our docs on how you can make Korbit work best for you and your team.
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.
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, but cc @betodealmeida to make sure I'm not missing some detail(s). Looks like the docs are up to date, which is nice :D
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #33764 +/- ##
===========================================
+ Coverage 60.48% 72.96% +12.47%
===========================================
Files 1931 558 -1373
Lines 76236 40293 -35943
Branches 8568 4223 -4345
===========================================
- Hits 46114 29399 -16715
+ Misses 28017 9803 -18214
+ Partials 2105 1091 -1014
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:
|
SUMMARY
Prior to this PR, when user choose RisingWave as the db engine, it will display
postgresql://...
as the sqlalchemy uri placeholder. However, this may cause issues when browsing the dataset because there are unsupported functionalities. Based on the official doc,risingwave:://...
should be used so that sqlalchemy-risingwave will be chosen to be the driver.This PR modifies the sqlalchemy_uri_placeholder for RisingWave engine and adds optional dependency for risingwave in
pyproject.toml
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION