eschutho commented on code in PR #44281:
URL: https://github.com/apache/superset/pull/44281#discussion_r4289386696
##########
superset/db_engine_specs/mssql.py:
##########
@@ -49,7 +55,7 @@
)
-class MssqlEngineSpec(BaseEngineSpec):
+class MssqlEngineSpec(BasicParametersMixin, BaseEngineSpec):
Review Comment:
Resolved by keeping Azure Synapse on the SQLAlchemy URI box (option (b) from
@rusackas's question), in f057a678f3:
- `AzureSynapseSpec.parameters_json_schema()` returns `None`, and
`/api/v1/database/available/` only adds `parameters` (i.e. offers the dynamic
form) when the spec returns a schema. Synapse therefore always gets the raw URI
box, whichever MSSQL drivers are installed, and the form can no longer produce
a driverless `mssql+pyodbc://` URI.
- `SybaseEngineSpec` (also a `MssqlEngineSpec` subclass, connecting through
an ODBC DSN) opts out the same way.
- The SQL Server form only builds `mssql+pymssql://` URIs and is only
offered when pymssql is installed.
I didn't try to make the builder emit `driver=`/`odbc_connect`: a usable
Synapse connection also needs the ODBC driver name,
`Encrypt`/`TrustServerCertificate`, and often an `Authentication` mode, which
the form doesn't capture. That can come later with Synapse-specific fields.
Covered by `test_mssql_subclasses_stay_on_uri_form`,
`test_available_keeps_azure_synapse_on_uri_form` (pyodbc-only and both drivers)
and the frontend test `Azure Synapse opens on the SQLAlchemy URI form`.
##########
tests/unit_tests/db_engine_specs/test_init.py:
##########
@@ -392,3 +392,107 @@ def test_get_available_engine_specs_with_denylist(mocker:
MockerFixture) -> None
)
available = get_available_engine_specs()
assert list(available.keys()) == [DatabricksNativeEngineSpec]
+
+
+class _MissingDBAPIDialect(DefaultDialect):
+ """A native dialect whose DBAPI is not installed."""
+
+ driver = "pyodbc"
+
+ @classmethod
+ def import_dbapi(cls) -> object:
+ raise ModuleNotFoundError("No module named 'pyodbc'")
+
+
+def test_get_available_engine_specs_detects_non_default_native_driver(
+ mocker: MockerFixture,
+) -> None:
+ """
+ With only pymssql installed, MSSQL is available with the pymssql driver
even
+ though SQLAlchemy's default ``mssql`` dialect (pyodbc) is missing.
+ """
+ import sqlalchemy.dialects
+ from sqlalchemy.dialects.mssql.pymssql import MSDialect_pymssql
+
+ from superset.db_engine_specs.mssql import AzureSynapseSpec,
MssqlEngineSpec
+
+ mocker.patch.object(sqlalchemy.dialects, "__all__", ["mssql"])
+ mocker.patch.object(
+ sqlalchemy.dialects.registry, "load", return_value=_MissingDBAPIDialect
+ )
+ mocker.patch.object(MSDialect_pymssql, "import_dbapi",
return_value=object())
+ mocker.patch(
+ "superset.db_engine_specs.load_engine_specs",
+ return_value=iter([MssqlEngineSpec, AzureSynapseSpec]),
+ )
+ mocker.patch("superset.db_engine_specs.entry_points", return_value=[])
+
+ available = get_available_engine_specs()
+
+ assert available[MssqlEngineSpec] == {"pymssql"}
+ assert available[AzureSynapseSpec] == {"pymssql"}
+
+
+def test_get_available_engine_specs_non_default_native_driver_not_installed(
+ mocker: MockerFixture,
+) -> None:
+ """
+ MSSQL has no drivers when neither pyodbc nor pymssql is installed.
+ """
+ import sqlalchemy.dialects
+ from sqlalchemy.dialects.mssql.pymssql import MSDialect_pymssql
+
+ from superset.db_engine_specs.mssql import MssqlEngineSpec
+
+ mocker.patch.object(sqlalchemy.dialects, "__all__", ["mssql"])
+ mocker.patch.object(
+ sqlalchemy.dialects.registry, "load", return_value=_MissingDBAPIDialect
+ )
+ mocker.patch.object(
+ MSDialect_pymssql,
+ "import_dbapi",
+ side_effect=ModuleNotFoundError("No module named 'pymssql'"),
+ )
+ mocker.patch(
+ "superset.db_engine_specs.load_engine_specs",
+ return_value=iter([MssqlEngineSpec]),
+ )
+ mocker.patch("superset.db_engine_specs.entry_points", return_value=[])
+
+ assert get_available_engine_specs()[MssqlEngineSpec] == set()
+
+
+def test_get_available_engine_specs_only_probes_native_backends(
+ mocker: MockerFixture,
+) -> None:
+ """
+ The ``default_driver`` probe is limited to SQLAlchemy's bundled dialects:
+ specs whose default driver was already detected, or whose backend is not a
+ native SQLAlchemy dialect, are not probed.
+ """
+ import sqlalchemy.dialects
+
+ from superset.db_engine_specs.mysql import MySQLEngineSpec
+ from superset.db_engine_specs.snowflake import SnowflakeEngineSpec
+
+ class MySQLDialect(DefaultDialect):
+ driver = "mysqldb"
+
+ @classmethod
+ def import_dbapi(cls) -> object:
+ return object()
+
+ mocker.patch.object(sqlalchemy.dialects, "__all__", ["mysql"])
+ mocker.patch.object(sqlalchemy.dialects.registry, "load",
return_value=MySQLDialect)
+ mocker.patch(
+ "superset.db_engine_specs.load_engine_specs",
+ return_value=iter([MySQLEngineSpec, SnowflakeEngineSpec]),
+ )
+ mocker.patch("superset.db_engine_specs.entry_points", return_value=[])
+ find_spec = mocker.patch("superset.db_engine_specs.find_spec")
Review Comment:
Fixed in 8888033d81: the mock is now `find_spec: MagicMock =
mocker.patch("superset.db_engine_specs.find_spec")`, with `MagicMock` imported
from `unittest.mock`.
--
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]
