aminghadersohi commented on code in PR #45843:
URL: https://github.com/apache/superset/pull/45843#discussion_r4417706754
##########
tests/unit_tests/commands/databases/validate_test.py:
##########
@@ -894,3 +894,80 @@ def test_ssh_tunnel_missing_message_is_interpolated(
assert missing_field_messages
assert all("%(missing)s" not in msg for msg in missing_field_messages)
assert any("server_port" in msg for msg in missing_field_messages)
+
+
[email protected](
+ "rebuilt_uri, expected_uri",
+ [
+ # unchanged native-form fields: the stored token is rehydrated
+ (
+ "databricks://token:XXXXXXXXXX@host:443"
+ "?catalog=main&http_path=%2Flegacy&schema=default",
+ "databricks://token:real-token@host:443/default",
+ ),
+ # a different warehouse keeps the masked token
+ (
+ "databricks://token:XXXXXXXXXX@host:443"
+ "?catalog=main&http_path=%2Fother&schema=default",
+ "databricks://token:XXXXXXXXXX@host:443"
+ "?catalog=main&http_path=%2Fother&schema=default",
+ ),
+ # a newly typed token is used as typed
+ (
+ "databricks://token:new-token@host:443"
+ "?catalog=main&http_path=%2Flegacy&schema=default",
+ "databricks://token:new-token@host:443"
+ "?catalog=main&http_path=%2Flegacy&schema=default",
+ ),
+ ],
+)
+def test_validate_converted_databricks_connection_token(
+ mocker: MockerFixture, rebuilt_uri: str, expected_uri: str
+) -> None:
+ """
+ A converted Databricks connection keeps legacy settings in ``connect_args``
+ while the native form rebuilds them into the URI query. Validation must
+ still test an unchanged connection with its stored token.
+ """
+ extra = json.dumps(
+ {"engine_params": {"connect_args": {"http_path": "/legacy", "catalog":
"main"}}}
+ )
+ existing = mocker.MagicMock()
+ existing.safe_sqlalchemy_uri.return_value = (
+ "databricks://token:XXXXXXXXXX@host:443/default"
+ )
+ existing.sqlalchemy_uri_decrypted =
"databricks://token:real-token@host:443/default"
+ existing.encrypted_extra = "{}"
+ existing.extra = extra
+ existing.ssh_tunnel = None
+
+ database = mocker.MagicMock()
+ with database.get_sqla_engine() as engine:
+ engine.dialect.do_ping.return_value = True
+
+ DatabaseDAO = mocker.patch( # noqa: N806
+ "superset.commands.database.validate.DatabaseDAO"
+ )
+ DatabaseDAO.find_by_id.return_value = existing
+ DatabaseDAO.validate_update_uniqueness.return_value = True
+ DatabaseDAO.build_db_for_connection_test.return_value = database
+
+ mocker.patch(
+ "superset.commands.database.validate.get_engine_spec",
+ return_value=mocker.MagicMock(
+ validate_parameters=mocker.MagicMock(return_value=[]),
Review Comment:
Fixed in a7badf30d3372822bba048f750f4babfca640d0a. I removed default_catalog
from the Python connector's required_parameters and made its Marshmallow form
field optional. `test_validate_converted_databricks_without_catalog` calls the
REAL validate_parameters and URI builder through
ValidateDatabaseParametersCommand, with only hostname/port probing and the
connection transport mocked. It verifies unchanged stored-token reuse without
typing a catalog. `test_native_catalog_is_optional_in_form_schema` pins the
browser schema too.
H1 is also covered by `keeps a created PostgreSQL field connection on step 3
without a saved schema` (including Finish retaining dynamic_form). Removing the
edit-only reducer guard produced five Jest failures; restoring it gives 94
passes. For H3 I chose the smaller option: hide Databricks (legacy) in the
available-databases API, while retaining saved-connection editing.
`test_available_hides_legacy_databricks` pins that picker response.
Searched: `rg -n 'ActionType.Fetched|parameters_schema.*properties'
superset-frontend/src/features/databases`; `rg -n
'required_parameters|default_catalog' superset/db_engine_specs`; `rg -n
'useAvailableDatabases|databases/available|get_available_engine_specs\('
superset-frontend/src superset`.
Siblings checked:
- superset/db_engine_specs/databricks.py:212 — fixed in
a7badf30d3372822bba048f750f4babfca640d0a
- superset/db_engine_specs/databricks.py:843 — fixed in
a7badf30d3372822bba048f750f4babfca640d0a
- superset-frontend/src/features/databases/DatabaseModal/index.tsx:576 —
fixed in a7badf30d3372822bba048f750f4babfca640d0a
- superset/databases/api.py:2003 — fixed in
a7badf30d3372822bba048f750f4babfca640d0a
- superset-frontend/src/views/CRUD/hooks.ts:932 — not affected: every picker
consumer receives the filtered API list.
- superset/views/base.py:588 — not affected: this registry caller only
checks Google Sheets installation, not picker options.
- superset/db_engine_specs/postgres.py:414 — not affected: no
default_catalog requirement; reducer coverage includes PostgreSQL, MySQL,
Databricks, and Google Sheets.
- superset/db_engine_specs/databricks.py:1063 — not affected: the legacy
required set contains database and extra, not default_catalog.
- superset/db_engine_specs/base.py:3402 — not affected: generic required
fields do not include default_catalog; the Databricks legacy spec also never
required it.
Remaining: invalid settings JSON blocking SQL Lab database-list loading is a
separate pre-existing issue. A broader API run has one baseline failure from an
unavailable BigQuery dialect; the focused suites above pass. Real warehouse
authentication and a local production frontend build were not exercised.
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]
