Skip to content

fix(databricks): restore the dynamic connection form - #45989

Open
mikebridge wants to merge 5 commits into
apache:masterfrom
mikebridge:databricks-dynamic-form
Open

mikebridge wants to merge 5 commits into
apache:masterfrom
mikebridge:databricks-dynamic-form

Conversation

@mikebridge

Copy link
Copy Markdown
Contributor

This draft carries #45911 by @Vitor-Avila forward while he is away. It merges master after #45843 and preserves both the Databricks dynamic form and the legacy-URI migration. The production-code conflict was the driver map: databricks remains the native default, while databricks-sql-python remains a saved-URI compatibility alias. The original summary below predates #45843; its statement that connection migration is follow-up work no longer applies because #45843 is in master. Existing DBS_AVAILABLE_DENYLIST entries using the old driver name continue to hide this option.

SUMMARY

The official databricks-sqlalchemy dialect reports its driver as databricks, but Superset advertises databricks-sql-python. The available-databases API only includes dynamic-form parameters when the spec's default driver is installed, so this mismatch omits the form schema.

Update the Python connector's default driver and driver mapping to databricks. This PR is limited to restoring the connection form; legacy-driver removal and connection migration are follow-up work.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before: with the official dialect installed, the Databricks connection form can appear blank because its parameters are missing from the API response.

After: the registered driver matches the spec, allowing the API to include the dynamic-form parameters.

TESTING INSTRUCTIONS

  1. Install Superset's Databricks extra with databricks-sqlalchemy 2.x.
  2. Open Settings → Database Connections → + Database, then select Databricks.
  3. Verify that the connection fields render. The Databricks entry from /api/v1/database/available/ should include default_driver: "databricks" and parameters.

Local validation: pre-commit passed; 61 focused Databricks, multi-cloud, and engine-discovery unit tests passed. OAuth and installed-dialect SQL-rendering tests were excluded because unrelated failures were also reproduced on the unchanged base. Live UI validation was not performed because the local server was not running.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

REPLACEMENT VALIDATION AND OPEN FEEDBACK

  • Joe's denylist thread: kept databricks-sql-python as a denylist alias for the renamed default. The red-first test failed for the old name before the fix and passed for both names afterward. No UPDATING.md entry is needed because old configuration keeps working.
  • The legacy-only-driver thread: the supported Databricks extra requires databricks-sqlalchemy 2.x. Its dialect registration and dialect both use databricks; databricks-sql-python is retained for saved-URI lookup, not emitted as an installed driver by that package. A legacy-only installed-driver set is outside the supported extra.
  • Replacement checks: 355 Databricks/API/discovery/migration/schema tests passed, 3 skipped; changed-file pre-commit passed. An additional suite had 254 passed and one pandas dtype failure in tests/unit_tests/models/core_test.py, whose code and test are unchanged from master. Live UI validation was not run.

@codecov

codecov Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.93%. Comparing base (3d9de97) to head (7889a20).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #45989   +/-   ##
=======================================
  Coverage   84.93%   84.93%           
=======================================
  Files        3077     3077           
  Lines      208205   208207    +2     
  Branches    49105    49105           
=======================================
+ Hits       176834   176836    +2     
  Misses      28002    28002           
  Partials     3369     3369           
Flag Coverage Δ
hive 34.19% <100.00%> (+<0.01%) ⬆️
mysql 52.17% <100.00%> (+<0.01%) ⬆️
postgres 52.19% <100.00%> (+<0.01%) ⬆️
presto 35.90% <100.00%> (+<0.01%) ⬆️
python 87.88% <100.00%> (+<0.01%) ⬆️
sqlite 51.93% <100.00%> (+<0.01%) ⬆️
unit 82.72% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mikebridge
mikebridge marked this pull request as ready for review November 3, 2026 19:56
@codeant-ai-for-open-source

Copy link
Copy Markdown
Contributor

CodeAnt  CodeAnt PR Risk

🟢  Low Risk

  • The PR appears safe to merge; no actionable issues were identified in the review.

Assessed commit: 7889a20e2bbd

@aminghadersohi
aminghadersohi self-requested a review November 3, 2026 21:17

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants