You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@superset.apache.org by "eschutho (via GitHub)" <gi...@apache.org> on 2026/09/15 05:21:35 UTC

[PR] feat(trino): add dynamic connection form for Trino [superset]

eschutho opened a new pull request, #44282:
URL: https://github.com/apache/superset/pull/44282

   ### SUMMARY
   
   Part of epic [sc-120993](https://app.shortcut.com/preset/story/120993) — migrating DB connections to the new dynamic connection form. Story: https://app.shortcut.com/preset/story/121000
   
   `TrinoEngineSpec` previously had no `parameters_schema`, so adding a Trino
   database showed the raw SQLAlchemy-URI box. This adds the dynamic ("new")
   connection form to Trino **only** by mixing in `BasicParametersMixin`.
   `/api/v1/database/available/` now returns dynamic form params (host, port,
   username, optional password, catalog, and an http/https encryption toggle)
   for the `trino` engine. `PrestoEngineSpec` / presto behavior is untouched.
   
   Design decisions / notes:
   
   - **`default_driver = "rest"`**: Trino's SQLAlchemy dialect registers a
     single `trino` dialect whose reported `driver` is `rest`. The `/available/`
     endpoint only advertises the form when `default_driver` is in the set of
     installed drivers, so it must be `"rest"`. However `trino+rest://` is **not**
     a loadable SQLAlchemy plugin, so `build_sqlalchemy_uri` is overridden to
     build a driverless `trino://` URI.
   - **Catalog mapping**: the Trino URI is `trino://user[:password]@host:port/catalog[/schema]`;
     the form's `database` field maps to the catalog (relabeled "Catalog name"
     via `TrinoParametersSchema`). This is compatible with the existing
     `adjust_engine_params` / `get_default_catalog` catalog-splitting logic in
     `PrestoBaseEngineSpec`.
   - **Encryption / http-https toggle**: the Trino dialect reads the scheme from
     `connect_args["http_scheme"]`, not from the URI. The toggle is round-tripped
     through a `protocol=https` query parameter (`encryption_parameters`) and
     translated into `connect_args["http_scheme"]` in `adjust_engine_params`
     (using `setdefault`, so an existing `http_scheme` from a server cert wins).
   - **Auth**: username is required, password optional; when a password is set the
     base mixin builds `trino://user:password@...`, which the dialect maps to
     `BasicAuthentication`.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   N/A — API/engine-spec change. Verified `/available/` advertises the form and
   the generated JSON schema (see testing instructions).
   
   ### TESTING INSTRUCTIONS
   
   - `pytest tests/unit_tests/db_engine_specs/test_trino.py` (125 passing,
     including new tests for `build_sqlalchemy_uri`, encryption round-trip and
     `adjust_engine_params` http_scheme translation).
   - Manually: add a new Trino database in the UI — the dynamic form now appears
     instead of the SQLAlchemy-URI box; the "Encrypt" toggle produces an
     `https` connection.
   
   ### ADDITIONAL INFORMATION
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
   - [x] Introduces new feature or API
   - [ ] Removes existing feature or API
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org