-
Notifications
You must be signed in to change notification settings - Fork 16.3k
feat(database): add cloudflare d1 support #36348
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.
Code Review Agent Run #dbd316
Actionable Suggestions - 2
-
docs/docs/configuration/databases.mdx - 2
- Incorrect Documentation · Line 57-57
- Incorrect Documentation · Line 363-375
Review Details
-
Files reviewed - 1 · Commit Range:
f53cd6c..cf3b59c- docs/docs/configuration/databases.mdx
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Default Agent You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
| | [Ascend.io](/docs/configuration/databases#ascendio) | `pip install impyla` | `ascend://{username}:{password}@{hostname}:{port}/{database}?auth_mechanism=PLAIN;use_ssl=true` | | ||
| | [Azure MS SQL](/docs/configuration/databases#sql-server) | `pip install pymssql` | `mssql+pymssql://UserName@presetSQL:TestPassword@presetSQL.database.windows.net:1433/TestSchema` | | ||
| | [ClickHouse](/docs/configuration/databases#clickhouse) | `pip install clickhouse-connect` | `clickhousedb://{username}:{password}@{hostname}:{port}/{database}` | | ||
| | [Cloudflare D1](/docs/configuration/databases#cloudflare-d1) | `pip install superset-engine-d1` | `d1://{cloudflare_account_id}:{cloudflare_api_token}@{cloudflare_d1_database_id}` | |
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.
This addition references a non-existent package 'superset-engine-d1' and GitHub repository 'sqlalchemy-cf-d1/superset-engine-d1', which could mislead users attempting to configure Cloudflare D1 support. Official Superset documentation and searches confirm no such integration exists yet.
Code Review Run #dbd316
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
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.
Package superset-engine-d1
Repository sqlalchemy-cf-d1/superset-engine-d1
| #### Cloudflare D1 | ||
|
|
||
| To use Cloudflare D1 with superset, install the [superset-engine-d1](https://github.com/sqlalchemy-cf-d1/superset-engine-d1) library. | ||
|
|
||
| ``` | ||
| pip install superset-engine-d1 | ||
| ``` | ||
|
|
||
| The expected connection string is formatted as follows: | ||
|
|
||
| ``` | ||
| d1://{cloudflare_account_id}:{cloudflare_api_token}@{cloudflare_d1_database_id} | ||
| ``` |
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.
The new Cloudflare D1 section links to a non-existent GitHub repository and package, potentially confusing users. No evidence of official D1 support in Superset exists based on current documentation and searches.
Code Review Run #dbd316
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
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.
Please find the referred packages and its respective code below:
Package superset-engine-d1
Repository sqlalchemy-cf-d1/superset-engine-d1
Side note: You can also find the code for the relevant dependencies with the organization.
|
Thanks for this! Pinging @betodealmeida for best practices advice here... having the db engine spec on pypi seems OK, but I wonder if it makes more sense (or not) to just add it to the Superset repo directly? ¯\_(ツ)_/¯ |
Thanks for this feedback, we actually had a pretty extensive conversation with @betodealmeida regarding external vs internal engine specs. We agreed that for a project like this where we may need to do more releases in the short term (for example if users run into needed features or bugs), then it's better to have the independent release cycle that comes with an external spec, as opposed to having to align our releases with those of Superset. The main drawback of an external spec is that if the Superset internal API changes then we will need to adjust our implementation too. However, this is a more hypothetical issue that we think is worth dealing with in return for our own release cycles. Our various libraries are also open-source, so the community is also able to help with such adjustments if needed. In the future after the implementation has been battle-tested by the community, there's also always the option to officially integrate it as an internal engine spec later on. That being said, we are always open to other thoughts on the matter. Feel free to let us know if there's anything we haven't considered. |
SUMMARY
This PR adds support for the D1 database in Superset via a new external engine package:
superset-engine-d1– Enables Superset to connect to D1 databases.sqlalchemy-d1anddbapi-d1– Underlying packages required by the engine, recursively installed as dependencies of the engine.The source code for these packages can be found in this organization
After installation, users can connect to D1 databases in Superset using the
d1engine spec. Example connection string:This PR also updates
docs/docs/configuration/databases.mdxwith the appropriate documentation for Cloudflare D1.TESTING INSTRUCTIONS
All packages have unit tests written. They can be installed by cloning the appropriate repositories and running
poetry run pytestTesting the functionality itself can be done by creating a Cloudflare account and following the connection process.
ADDITIONAL INFORMATION