Skip to content

Security, metadata and mcp sources open two connections; close-on-failure has drifted across the service bases #30173

Description

@IceS2

The source-owns-BaseConnection work is complete for the 9 standard service types (#29870 dashboard, #30094 database, #30148 pipeline, #30133 remaining). Two loose ends are left on the OSS side.

1. Sources that still open two connections

SecurityServiceSource.__init__ builds two clients: one through the owner-discarding get_connection seam and a second one through its get_client hook. It then calls self.client.test_connection() and discards the result, so a failing connection never raises and ingestion proceeds against a broken client.

The metadata (atlas, amundsen, alationsink) and mcp sources have no service base, but each carries the same pattern in its own __init__: a client is built through the seam, then test_connection_common builds a second one to test with.

All of these should own a single BaseConnection and reuse it for the test step, like the migrated bases do.

2. close_on_failure

Every service base re-derives try: self.test_connection() except: <dispose>; raise in its constructor, and the shape has drifted: most call self.close(), pipeline disposes self._connection directly.

self.close() is the wrong teardown for a constructor failure. It is successful-run teardown, and it is polymorphic — it dispatches to subclass overrides that touch attributes set after super().__init__() returns (airflow's self.session, databrickspipeline's self._table_lookup_cache), so an AttributeError masks the connection error the user needs to see. saperp's override goes further and closes the OpenMetadata client.

A single close_on_failure context manager in source/connections.py releases only the owner — the only resource in flight when the test step runs.

Scope

  • Migrate security_service.py plus the four metadata/mcp leaves; audit drives/ (dead package, N/A).
  • Add close_on_failure and apply it to every base and leaf that owns a connection.
  • Fix airflow and openlineage close(), which never call super().close() and so never dispose the owner.

Additive and backward-compatible: connection_obj, test_connection_common's signature and the get_connection/import_connection_fn seam are all preserved for Collate and custom connectors.

Tracked PR: #30174

Metadata

Metadata

Assignees

Labels

Type

Fields

No fields configured for Task.

Projects

Status
Done ✅
Status
Done ✅

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions