Fixes #29765: refactor(databricks): migrate test-connection to declarative @check framework

[IceS2]

Fixes #29765

Migrates the Databricks connector's test-connection off the legacy test_connection_steps helper onto the declarative @check / ChecksProvider / ErrorPack framework.

What changed

• databricks/connection.py: removed the DatabricksConnection.test_connection override; added DatabricksChecks (a ChecksProvider) with one @check per seeded step and a checks() method. _get_client now registers engine.dispose on close. The DatabricksEngineWrapper and lazy catalog scoping are preserved; every step builds its statement inside its @check method, so nothing touches the network before the CheckAccess gate.
• 13 steps: CheckAccess (TCP preflight to the workspace host:443 + SELECT 1), GetDatabases (catalogs), GetSchemas, GetTables, GetViews, GetQueries, GetViewDefinitions, GetCatalogTags, GetSchemaTags, GetTableTags, GetColumnTags, GetTableLineage, GetColumnLineage.
• DATABRICKS_ERRORS: an ErrorPack keyed on stable databricks-sql / thrift message tokens (invalid/expired token, forbidden, PERMISSION_DENIED/INSUFFICIENT_PERMISSIONS, TABLE_OR_VIEW_NOT_FOUND, SCHEMA_NOT_FOUND, catalog "does not exist"), folding in the shared NETWORK_ERRORS pack.
• checks/database.py: added the 7 greenfield members (GetViewDefinitions, GetCatalogTags, GetSchemaTags, GetTableTags, GetColumnTags, GetTableLineage, GetColumnLineage) to the shared DatabaseStep enum. No existing shared logic was touched.
• databricks.json: marked CheckAccess with "category": "ConnectionGate" so the runner short-circuits on a failed gate.
• Tests: new colocated tests/unit/source/database/databricks/test_connection.py (step coverage, error-pack classification, gate preflight, lazy construction); updated the topology test's wrapper assertions to check the materialized rows and dropped the obsolete test_connection_steps wiring test.

[gitar-bot]

💡 Quality: "+" suffix in _summarize misleads: list ops are never capped

_summarize appends a + when count >= DEFAULT_SAMPLE_ROWS (100), with the comment stating the + marks a capped sample so the figure is not read as an exact total. However _summarize is only used via _list, whose backing wrapper methods (get_catalogs, get_schemas, get_tables, get_views) all return uncapped fetchall() results. The capping (fetchmany(max_rows)) only exists in run_sql, whose checks use their own summarize lambdas that ignore rows and never call _summarize. So a schema with exactly 100+ tables/views/schemas will be reported as e.g. 100+ tables enumerated, wrongly implying truncation when it is in fact the exact total. Either cap the wrapper listings to match the summary semantics, or drop the + suffix (and comment) from _summarize since these counts are always exact.

Drop the misleading '+' suffix since _list results are uncapped exact counts.:

def _summarize(rows: Sequence[object], noun: str) -> str:
    """``N <noun>s enumerated`` or ``no <noun>s enumerated``."""
    count = len(rows)
    if not count:
        return f"no {noun}s enumerated"
    plural = noun if count == 1 else f"{noun}s"
    return f"{count} {plural} enumerated"

• Apply fix

_{Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎}

[gitar-bot]

Code Review 👍 Approved with suggestions 0 resolved / 1 findings

Migrates Databricks connection testing to the declarative @check framework with 13 granular steps and centralized error handling. Remove the misleading '+' suffix in the _summarize logic, as list operations in this context are never capped.

💡 Quality: "+" suffix in _summarize misleads: list ops are never capped

📄 ingestion/src/metadata/ingestion/source/database/databricks/connection.py:139-148 📄 ingestion/src/metadata/ingestion/source/database/databricks/connection.py:283-293

_summarize appends a + when count >= DEFAULT_SAMPLE_ROWS (100), with the comment stating the + marks a capped sample so the figure is not read as an exact total. However _summarize is only used via _list, whose backing wrapper methods (get_catalogs, get_schemas, get_tables, get_views) all return uncapped fetchall() results. The capping (fetchmany(max_rows)) only exists in run_sql, whose checks use their own summarize lambdas that ignore rows and never call _summarize. So a schema with exactly 100+ tables/views/schemas will be reported as e.g. 100+ tables enumerated, wrongly implying truncation when it is in fact the exact total. Either cap the wrapper listings to match the summary semantics, or drop the + suffix (and comment) from _summarize since these counts are always exact.

Drop the misleading '+' suffix since _list results are uncapped exact counts.

def _summarize(rows: Sequence[object], noun: str) -> str:
    """``N <noun>s enumerated`` or ``no <noun>s enumerated``."""
    count = len(rows)
    if not count:
        return f"no {noun}s enumerated"
    plural = noun if count == 1 else f"{noun}s"
    return f"{count} {plural} enumerated"

🤖 Prompt for agents

Code Review: Migrates Databricks connection testing to the declarative @check framework with 13 granular steps and centralized error handling. Remove the misleading '+' suffix in the `_summarize` logic, as list operations in this context are never capped.

1. 💡 Quality: "+" suffix in _summarize misleads: list ops are never capped
   Files: ingestion/src/metadata/ingestion/source/database/databricks/connection.py:139-148, ingestion/src/metadata/ingestion/source/database/databricks/connection.py:283-293

   `_summarize` appends a `+` when `count >= DEFAULT_SAMPLE_ROWS` (100), with the comment stating the `+` marks a capped sample so the figure is not read as an exact total. However `_summarize` is only used via `_list`, whose backing wrapper methods (`get_catalogs`, `get_schemas`, `get_tables`, `get_views`) all return uncapped `fetchall()` results. The capping (`fetchmany(max_rows)`) only exists in `run_sql`, whose checks use their own summarize lambdas that ignore rows and never call `_summarize`. So a schema with exactly 100+ tables/views/schemas will be reported as e.g. `100+ tables enumerated`, wrongly implying truncation when it is in fact the exact total. Either cap the wrapper listings to match the summary semantics, or drop the `+` suffix (and comment) from `_summarize` since these counts are always exact.

   Fix (Drop the misleading '+' suffix since _list results are uncapped exact counts.):
   def _summarize(rows: Sequence[object], noun: str) -> str:
       """``N <noun>s enumerated`` or ``no <noun>s enumerated``."""
       count = len(rows)
       if not count:
           return f"no {noun}s enumerated"
       plural = noun if count == 1 else f"{noun}s"
       return f"{count} {plural} enumerated"

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact

gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}