feat: return parameters only for DB with default driver #14803
Conversation
betodealmeida
changed the title
betodealmeida
force-pushed
the
ch15642
branch
3 times, most recently
from
e77bc84
to
08b2762
Compare
| available_databases.sort( | ||
| key=lambda payload: preferred_databases.index(payload["engine"]) | ||
| if payload["engine"] in preferred_databases | ||
| else len(preferred_databases) |
There was a problem hiding this comment.
This was sorting the preferred DBs correctly, but doing a non-deterministic sort on the non-preferred. 🤦
Is SSL the only difference between the default and non default driver? Could we show them the form but not give them the option to turn on ssl (like we do for admins who enforce ssl but the opposite)? |
Right, but there could potentially be other parameters that are driver-dependent — maybe a MySQL driver uses client certificates instead of username/password — so I think we should only show the form if the default driver is installed. Also, if we show the form when the default driver is not installed, and the user has more than one non-default drivers installed, this raises the problem of which one we should use. Eg, the default driver for MySQL is I think users in cases like this probably know better, and so they can just the SQLAlchemy URI to configure the DB with a non-default driver. |
Codecov Report
@@ Coverage Diff @@
## master #14803 +/- ##
==========================================
- Coverage 77.38% 77.35% -0.04%
==========================================
Files 962 962
Lines 49174 49192 +18
Branches 6184 6184
==========================================
- Hits 38055 38051 -4
- Misses 10917 10939 +22
Partials 202 202
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
* WIP * Fix sorting of DBs
* WIP * Fix sorting of DBs
* WIP * Fix sorting of DBs
mistercrunch
added
🏷️ bot
SUMMARY
This PR improves the
/api/v1/database/available/endpoint:mysqldbandmysqlconnectorfor MySQL, in my system).mysqldbfor MySQL).mysqldbis not installed butmysqlconectoris, don't show the parameters for MySQL).The reason for (3) is that if the user doesn't have the default driver installed we can't enable SSL for them. For example, for MySQL the engine spec can only enable it for the default
mysqldbdriver (see).If the user wants to use the non-default driver they need to use the old method via the SQLAlchemy URI. In that case we don't show the
parametersfor the engine spec, so that the modal should only allow the DB to be configured via the URI.(1) and (2) will not be used by the modal, but are useful for debugging and troubleshooting.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A
TESTING INSTRUCTIONS
Updated unit tests.
Also tested manually. On my machine, with:
The response in
/api/v1/database/available/is:{ "databases": [ { "available_drivers": [ "mysqlconnector", "mysqldb" ], "default_driver": "mysqldb", "engine": "mysql", "name": "MySQL", "parameters": { "properties": { "database": { "description": "Database name", "type": "string" }, "encryption": { "description": "Use an encrypted connection to the database", "type": "boolean" }, "host": { "description": "Hostname or IP address", "type": "string" }, "password": { "description": "Password", "nullable": true, "type": "string" }, "port": { "description": "Database port", "format": "int32", "type": "integer" }, "query": { "additionalProperties": {}, "description": "Additional parameters", "type": "object" }, "username": { "description": "Username", "nullable": true, "type": "string" } }, "required": [ "database", "host", "port", "username" ], "type": "object" }, "preferred": false, "sqlalchemy_uri_placeholder": "mysql://user:password@host:port/dbname[?key=value&key=value...]" }, { "available_drivers": [ "psycopg2" ], "default_driver": "psycopg2", "engine": "postgresql", "name": "PostgreSQL", "parameters": { "properties": { "database": { "description": "Database name", "type": "string" }, "encryption": { "description": "Use an encrypted connection to the database", "type": "boolean" }, "host": { "description": "Hostname or IP address", "type": "string" }, "password": { "description": "Password", "nullable": true, "type": "string" }, "port": { "description": "Database port", "format": "int32", "type": "integer" }, "query": { "additionalProperties": {}, "description": "Additional parameters", "type": "object" }, "username": { "description": "Username", "nullable": true, "type": "string" } }, "required": [ "database", "host", "port", "username" ], "type": "object" }, "preferred": true, "sqlalchemy_uri_placeholder": "postgresql://user:password@host:port/dbname[?key=value&key=value...]" }, { "available_drivers": [ "pysqlite" ], "engine": "sqlite", "name": "SQLite", "preferred": false }, { "available_drivers": [ "vertica_python" ], "engine": "vertica", "name": "Vertica", "preferred": false }, { "available_drivers": [ "rest" ], "engine": "presto", "name": "Presto", "preferred": true }, { "available_drivers": [ "rest" ], "engine": "druid", "name": "Apache Druid", "preferred": false }, { "available_drivers": [ "bigquery" ], "default_driver": "bigquery", "engine": "bigquery", "name": "Google BigQuery", "parameters": { "properties": { "credentials_info": { "description": "Contents of BigQuery JSON credentials.", "type": "string", "x-encrypted-extra": true } }, "type": "object" }, "preferred": false, "sqlalchemy_uri_placeholder": "bigquery://{project_id}" }, { "available_drivers": [ "turbodbc" ], "engine": "mssql", "name": "Microsoft SQL", "preferred": false }, { "available_drivers": [ "apsw" ], "engine": "gsheets", "name": "Google Sheets", "preferred": false } ] }ADDITIONAL INFORMATION