aminghadersohi commented on code in PR #45843:
URL: https://github.com/apache/superset/pull/45843#discussion_r4417702423
##########
superset/models/core.py:
##########
@@ -435,14 +435,15 @@ def parameters(self) -> dict[str, Any]:
# the sqlalchemy_uri
# When returning the parameters we should use the masked SQLAlchemy
URI and the
# masked ``encrypted_extra`` to prevent exposing sensitive credentials.
- masked_uri = make_url_safe(self.sqlalchemy_uri)
+ # An unparseable URI raises here rather than yielding empty parameters.
+ make_url_safe(self.sqlalchemy_uri)
Review Comment:
Fixed in a7badf30d3372822bba048f750f4babfca640d0a. I moved parsing and
masked-extra extraction inside the existing parameters fallback and made an
unavailable default driver serialize even when its plugin cannot be loaded.
`test_connection_details_allow_uri_repair` covers malformed ports, an
unparseable URI, and an uninstalled driver through the real
`DatabaseConnectionSchema`, preserving the URI for repair.
Searched: `rg -n 'def parameters|def
driver|parameters_schema|DatabaseConnectionSchema' superset/models/core.py
superset/databases`.
Siblings checked:
- superset/models/core.py:425 — fixed in
a7badf30d3372822bba048f750f4babfca640d0a
- superset/models/core.py:437 — fixed in
a7badf30d3372822bba048f750f4babfca640d0a
- superset/models/core.py:459 — not affected: parameters_schema already
returns an empty schema on extraction failure.
- superset/databases/api.py:347 — fixed in
a7badf30d3372822bba048f750f4babfca640d0a
Remaining: malformed settings JSON can still prevent SQL Lab database-list
loading. I left that pre-existing behavior separate rather than silently
discarding operational connection settings.
Verification: regression selections failed against
54f95ada005a48e5baa6ca86ea5d6f7501d8837c and pass with
a7badf30d3372822bba048f750f4babfca640d0a. Focused backend suites: 416 passed, 1
skipped; picker API test: 1 passed; DatabaseModal/reducer Jest: 94 passed.
Branch-scoped pre-commit passed, including mypy.
##########
superset-frontend/src/features/databases/DatabaseModal/index.tsx:
##########
@@ -787,6 +793,16 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps>
= ({
) ||
{};
+ // A saved driver's schema can remain usable even when that driver is no
longer
+ // installed. Do not replace it with another driver's form for the same
engine.
+ const dbModel: DatabaseForm =
+ isEditMode && db?.parameters_schema?.properties
+ ? {
+ ...availableDbModel,
+ parameters: db.parameters_schema as DatabaseForm['parameters'],
Review Comment:
Fixed in a7badf30d3372822bba048f750f4babfca640d0a. I retained the saved
compatibility schema and corrected its legacy URI builder to emit
`databricks+connector://`, including its placeholder. I did not relax the
driver-identity check. `test_legacy_form_preserves_connector_scheme` and
`test_validate_legacy_databricks_form_keeps_masked_token` exercise the real
legacy builder and real validation, including stored-token reuse.
Searched: `rg -n 'class .*Databricks|def
build_sqlalchemy_uri|parameters_schema' superset/db_engine_specs/databricks.py
superset-frontend/src/features/databases/DatabaseModal/index.tsx`.
Siblings checked:
- superset/db_engine_specs/databricks.py:1084 — fixed in
a7badf30d3372822bba048f750f4babfca640d0a
- superset-frontend/src/features/databases/DatabaseModal/index.tsx:800 — not
affected: the saved schema remains authoritative; its builder no longer changes
the driver.
- superset/db_engine_specs/databricks.py:861 — not affected: the native
Python connector intentionally builds databricks URIs.
- superset/db_engine_specs/databricks.py:1200 — not affected: the ODBC and
Hive specs do not use the compatibility field-form builder.
Verification: regression selections failed against
54f95ada005a48e5baa6ca86ea5d6f7501d8837c and pass with
a7badf30d3372822bba048f750f4babfca640d0a. Focused backend suites: 416 passed, 1
skipped; picker API test: 1 passed; DatabaseModal/reducer Jest: 94 passed.
Branch-scoped pre-commit passed, including mypy.
##########
superset/commands/database/utils.py:
##########
@@ -58,7 +80,12 @@ def uri_identity_changed(existing_uri: str | None,
submitted_uri: str | None) ->
# An unparseable URI cannot be compared: treat it as a change so a
# stored secret never survives onto it.
return True
- return stored != incoming
+ if stored == incoming:
+ return False
+ engine_spec = Database.get_db_engine_spec(stored)
+ return engine_spec.get_connection_identity(
+ stored, _connect_args(existing_extra)
+ ) != engine_spec.get_connection_identity(incoming,
_connect_args(submitted_extra))
Review Comment:
Fixed in a7badf30d3372822bba048f750f4babfca640d0a at the legacy form
builder, rather than treating different drivers as the same credential
destination. The builder preserves `+connector`, so unchanged fields compare
equal in `uri_identity_changed`. `test_legacy_form_preserves_connector_scheme`
checks this comparison directly;
`test_validate_legacy_databricks_form_keeps_masked_token` verifies that
validation uses the stored token instead of the mask. The changed-destination
rejection remains in place.
Searched: `rg -n 'uri_identity_changed\(|def build_sqlalchemy_uri'
superset/commands/database superset/db_engine_specs/databricks.py`.
Siblings checked:
- superset/db_engine_specs/databricks.py:1084 — fixed in
a7badf30d3372822bba048f750f4babfca640d0a
- superset/commands/database/utils.py:60 — not affected: its
different-driver rejection is intentionally retained; the caller no longer
silently switches schemes.
- superset/commands/database/validate.py:252 — fixed in
a7badf30d3372822bba048f750f4babfca640d0a
- superset/commands/database/update.py:307 — not affected: the unchanged
legacy form now emits the stored scheme; destination-change checks remain
active.
- superset/commands/database/importers/v1/utils.py:49 — not affected:
imports compare explicitly supplied URIs, not a field-form rebuild.
Verification: regression selections failed against
54f95ada005a48e5baa6ca86ea5d6f7501d8837c and pass with
a7badf30d3372822bba048f750f4babfca640d0a. Focused backend suites: 416 passed, 1
skipped; picker API test: 1 passed; DatabaseModal/reducer Jest: 94 passed.
Branch-scoped pre-commit passed, including mypy.
--
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]
