diff --git a/HISTORY.rst b/HISTORY.rst index 191e613..78eed42 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -14,6 +14,13 @@ History * ``RunInfo`` and ``UnfinishedRun`` gain a required ``project`` field: the run's project, taken from its ruleset's project when the run starts. * Added ``connect_timeout_seconds`` and ``read_timeout_seconds`` to the instance config models. +* Added an optional ``instance`` field to ``DatabaseConnectionConfig``: the SQL Server named instance, + taken from the Instance input in the connection form's advanced settings. + It round-trips through ``list_connections`` and ``create_or_update_connection``, + and is sent only for ``mssql`` and ``mssql_linked``, the two engines that have it on the server. +* ``DatabaseConnectionConfig.port`` is now optional. + A SQL Server connection that names an instance is reached through the SQL Server Browser and stores no port, + so a connection saved that way previously failed to deserialize. Requires server version 3.26.20 diff --git a/datamasque/client/models/connection.py b/datamasque/client/models/connection.py index 5bec97b..9ddbddc 100644 --- a/datamasque/client/models/connection.py +++ b/datamasque/client/models/connection.py @@ -376,7 +376,9 @@ class DatabaseConnectionConfig(ConnectionConfig): """ host: str - port: int + # The server leaves the port unset when the connection reaches its database another way: a SQL + # Server named instance resolved by the SQL Server Browser, or an engine with a default port. + port: Optional[int] = None database: str user: str password: Optional[str] = None @@ -384,6 +386,9 @@ class DatabaseConnectionConfig(ConnectionConfig): engine_options: Optional[dict] = None db_schema: Optional[str] = Field(default=None, alias="schema") data_encoding: Optional[str] = None + # The SQL Server named instance to connect to, in place of a port. Only `mssql` and + # `mssql_linked` carry it on the server, so it is pruned for the rest. + instance: Optional[str] = None is_read_only: bool = False s3_bucket_name: Optional[str] = None s3_redshift_iam_role: Optional[str] = None @@ -443,6 +448,8 @@ def _serialize(self, handler: Callable) -> dict: d.pop("s3_redshift_iam_role", None) if db_type not in _AWS_TAGGABLE_DATABASE_TYPES: d.pop("iam_role_arn", None) + if db_type not in {DatabaseType.mssql, DatabaseType.mssql_linked}: + d.pop("instance", None) if not d.get("engine_options"): d.pop("engine_options", None) return d diff --git a/tests/test_connections.py b/tests/test_connections.py index cbc3f7d..9469bdc 100644 --- a/tests/test_connections.py +++ b/tests/test_connections.py @@ -1594,6 +1594,117 @@ def test_redshift_connection_keeps_its_cluster_s3_role_separate_from_its_tagging assert api_dict["iam_role_arn"] == "arn:aws:iam::119836602066:role/tagger" +def _sql_server_connection_json(**extra) -> dict: + """A SQL Server connection as `GET /api/connections/` returns it, with no port of its own.""" + return { + "id": "9f1d2c4e-5b6a-4f8d-9c0e-1a2b3c4d5e6f", + "name": "mssql_named_instance", + "mask_type": "database", + "db_type": "mssql", + "host": "sql-server.example.com", + "database": "mssqldb", + "user": "mssqluser", + "schema": "dbo", + "is_read_only": False, + **extra, + } + + +def test_mssql_connection_read_from_the_api_keeps_its_named_instance_and_blank_port(): + """A named instance is reached through the SQL Server Browser, so the server stores no port.""" + conn = DatabaseConnectionConfig.model_validate(_sql_server_connection_json(instance="SQLEXPRESS")) + + assert conn.instance == "SQLEXPRESS" + assert conn.port is None + + +def test_mssql_connection_read_from_the_api_without_an_instance_has_none(): + conn = DatabaseConnectionConfig.model_validate(_sql_server_connection_json(port=1433)) + + assert conn.instance is None + assert conn.port == 1433 + + +def test_mssql_connection_round_trips_its_named_instance_back_to_the_api(): + conn = DatabaseConnectionConfig.model_validate(_sql_server_connection_json(instance="SQLEXPRESS")) + + api_dict = conn.model_dump(exclude_none=True, by_alias=True, mode="json") + + assert api_dict["instance"] == "SQLEXPRESS" + assert "port" not in api_dict + + +def test_create_mssql_connection_sends_the_named_instance(client): + config = DatabaseConnectionConfig( + name="mssql_named_instance", + host="sql-server.example.com", + database="mssqldb", + user="mssqluser", + password="secret", + database_type=DatabaseType.mssql, + instance="SQLEXPRESS", + ) + + with requests_mock.Mocker() as m: + m.get("http://test-server/api/connections/", json=[], status_code=200) + m.post("http://test-server/api/connections/", json={"id": "mssql-id-1"}, status_code=201) + + result = client.create_or_update_connection(config) + + assert result.id == ConnectionId("mssql-id-1") + sent = m.last_request.json() + assert sent["instance"] == "SQLEXPRESS" + assert sent["db_type"] == "mssql" + + +def test_mssql_linked_server_connection_keeps_its_named_instance(): + """A linked-server connection names the instance that holds the linked server.""" + conn = MssqlLinkedServerConnectionConfig( + name="mssql_linked", + host="sql-server.example.com", + database="mssqldb", + user="mssqluser", + password="secret", + database_type=DatabaseType.mssql_linked, + linked_server="other.db.dbo", + instance="SQLEXPRESS", + ) + + api_dict = conn.model_dump(exclude_none=True, by_alias=True, mode="json") + + assert api_dict["instance"] == "SQLEXPRESS" + + +@pytest.mark.parametrize( + "database_type", + [ + DatabaseType.postgres, + DatabaseType.mysql, + DatabaseType.oracle, + DatabaseType.db2_luw, + DatabaseType.redshift, + ], +) +def test_database_connection_omits_the_named_instance_for_engines_without_one(database_type): + """These engines have no `instance` field on the server, so sending one would be rejected.""" + extra = ( + {"s3_bucket_name": "bucket", "s3_redshift_iam_role": "arn:aws:iam::123456789012:role/redshift-s3"} + if database_type is DatabaseType.redshift + else {} + ) + conn = _database_connection(database_type, instance="SQLEXPRESS", **extra) + + api_dict = conn.model_dump(exclude_none=True, by_alias=True, mode="json") + + assert "instance" not in api_dict + + +def test_database_connection_omits_the_named_instance_when_unset(): + conn = _database_connection(DatabaseType.mssql) + + assert "instance" not in conn.model_dump(exclude_none=True, by_alias=True, mode="json") + + @pytest.mark.parametrize( ("database_type", "message"), [