eschutho opened a new pull request, #44283: URL: https://github.com/apache/superset/pull/44283
### SUMMARY Part of epic [sc-120993](https://app.shortcut.com/preset/story/120993) (migrate DB connections to the new dynamic connection form). Story: https://app.shortcut.com/preset/story/120997 Superset ships two ClickHouse engine specs: - `ClickHouseEngineSpec` — backend key `clickhouse`, the `clickhouse-sqlalchemy` driver. **Legacy**, no `parameters_schema`, so its connections still render the free-form SQLAlchemy URI editor. - `ClickHouseConnectEngineSpec` — backend key `clickhousedb`, default driver `connect` (`clickhouse-connect`). Defines a `parameters_schema` and renders the **new dynamic connection form**. The engine spec is selected by the URI's backend name (`Database.db_engine_spec` reads `url.get_backend_name()`), so this is a pure data migration that rewrites the `sqlalchemy_uri` scheme of legacy `clickhouse` rows to `clickhousedb+connect://`. After it runs, an affected connection resolves to `ClickHouseConnectEngineSpec` and exposes the new form. No `parameters_schema` change — this is an engine-key/data migration only. **Verified:** `get_engine_spec("clickhousedb", "connect")` → `ClickHouseConnectEngineSpec` (`parameters_schema` present); `get_engine_spec("clickhouse", …)` → legacy `ClickHouseEngineSpec` (none). #### Scheme / port mapping | Legacy scheme | Driver / protocol | Default port | Rewritten to | Port | |---|---|---|---|---| | `clickhouse://`, `clickhouse+http://` | clickhouse-sqlalchemy, HTTP | 8123 | `clickhousedb+connect://` | **8123 (unchanged)** | | `clickhouse+native://`, `clickhouse+asynch://` | native TCP | 9000 | `clickhousedb+connect://` | **8123** | `clickhouse-connect` is **HTTP-only** — it cannot speak the native TCP protocol — so native connections must also move off port 9000. When the native port is the default `9000` (or absent, which implies 9000) it is rewritten to the HTTP default `8123`. A **non-default** native port cannot be mapped to an HTTP port without knowing the server config, so those rows are **left on the legacy driver** for manual migration rather than pointed at a guessed port. Operators should confirm the HTTP interface is enabled on rewritten native connections. For HTTP rows only the scheme changes; host, port, credentials, database and query string (incl. `secure=…`) are preserved byte-for-byte. The password — inline in the URI or in `encrypted_extra` — is preserved either way, since the URI is edited in place and `encrypted_extra` is never read. The migration also **merges the two open revision heads** on master (`88a01c781622` index_ab_user_lower_username and `c7f53d184ea2` coordinate_purge_audit_pruning, forked at `7e2c9a4f1b83`) so the tree keeps a single head. #### Rollback `downgrade()` is an intentional, documented **no-op**. After upgrade a migrated row is byte-for-byte indistinguishable from a connection created directly on the `clickhousedb` key (both render as `clickhousedb+connect://…`), so a blind reversal would also revert genuine `clickhousedb` connections onto the legacy driver and break them. The migrated connections keep working after a code rollback regardless, because `ClickHouseConnectEngineSpec` predates this migration — no data restoration is required. `upgrade()` is idempotent (rows already on `clickhousedb` are skipped). ### TESTING INSTRUCTIONS - Unit tests: `pytest tests/unit_tests/migrations/test_migrate_clickhouse_to_clickhousedb.py` (18 cases — HTTP/native rewrites, port remap, skipped rows, idempotency, no-op downgrade). - Manual: create a DB connection with a `clickhouse://user:pass@host:8123/db` URI (legacy free-form editor), run `superset db upgrade`, then edit the connection — it now renders the new dynamic ClickHouse form. Verify a `clickhouse+native://host:9000/db` row becomes `clickhousedb+connect://host:8123/db`, and a `clickhouse+native://host:9440/db` row is left untouched. ### ADDITIONAL INFORMATION - [ ] Has associated issue: - [ ] Required feature flags: - [ ] Changes UI - [x] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [x] Migration is atomic, supports rollback & is backwards-compatible - [x] Confirm DB migration upgrade and downgrade tested - [x] Runtime estimates and downtime expectations provided - [ ] Introduces new feature or API - [ ] Removes existing feature or API Runtime: a single paginated `UPDATE` over `dbs` rows whose URI begins with `clickhouse` (fleet ≈ 30 rows); negligible runtime, no downtime, no locking of unrelated tables. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
