fix(ingestion): detect missing Lake Formation grants in Athena test connection - #29515
Conversation
…onnection Athena test connection reported all steps green even when the IAM role lacked Lake Formation DESCRIBE/SELECT grants, so the run "succeeded" but ingested 0 tables. AWS Lake Formation silently filters results (pyathena even converts the underlying ClientError into an empty list), so inspector.get_table_names() returned empty and GetTables passed. The old executor also probed only the first schema, not the schemas the configured schemaFilterPattern targets. GetTables now probes the schemas schemaFilterPattern would target (capped at MAX_SCHEMAS_TO_PROBE) and raises when no tables are readable in any of them, with a message pointing at Lake Formation grants on the catalog. It passes as soon as any one schema is readable. GetViews uses the same selection but never raises on empty. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
✅ PR checks passedThe linked issue has a description and all required Shipping project fields set. Thanks! |
github-actions
Bot
added
Ingestion
safe to test
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Improves the Athena connector’s Test Connection behavior to detect the “silent empty list” case caused by missing AWS Lake Formation grants, preventing false-green results that later lead to ingesting 0 tables with no signal.
Changes:
- Added schema targeting logic (honoring
schemaFilterPattern) and capped probing to avoid exhausting the test-connection timeout. - Updated
GetTablesto fail when no tables are readable across targeted schemas, and updatedGetViewsto probe similarly without failing on empty views. - Added unit tests covering missing grants behavior, schema filtering, and the probing cap.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| ingestion/src/metadata/ingestion/source/database/athena/connection.py | Adds targeted-schema selection + capped probing; makes GetTables raise on “zero readable tables” while keeping GetViews non-failing on empty results. |
| ingestion/tests/unit/source/database/athena/test_connection.py | Adds unit tests to validate the new GetTables/GetViews behaviors, schema filter honoring, and the max-schemas cap. |
Comments suppressed due to low confidence (1)
ingestion/src/metadata/ingestion/source/database/athena/connection.py:164
- The PR description mentions updating the Athena Test Connection definition’s
GetTables.errorMessageto mention Lake Formation, but the current definition file still has the generic privilege message. Since the UI result stores the one-liner inTestConnectionStepResult.message(fromerrorMessage) separately from the exception text, users may still see an unhelpful message unless the definition is updated.
test_fn = {
"CheckAccess": partial(test_connection_engine_step, engine),
"GetSchemas": partial(execute_inspector_func, engine, "get_schema_names"),
"GetTables": custom_executor_for_table,
"GetViews": custom_executor_for_view,
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Jest test CoverageUI tests summary
|
|
🟡 Playwright Results — all passed (14 flaky)✅ 4457 passed · ❌ 0 failed · 🟡 14 flaky · ⏭️ 88 skipped
🟡 14 flaky test(s) (passed on retry)
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
Memoize the targeted-schema listing so GetTables and GetViews share a single get_schema_names() round-trip instead of one each. Stop collecting schemas once MAX_SCHEMAS_TO_PROBE is reached rather than slicing after the fact. Replace the any()-for-side-effect view probe with an explicit for/break loop to avoid the discarded-return anti-pattern (B015). Soften the GetTables error message to note the catalog may legitimately be empty, and document the empty/view-only catalog as a by-design failure. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Code Review ✅ Approved 2 resolved / 2 findingsImproves Athena connection testing by replacing naive single-schema probes with a filter-aware, capped multi-schema scan. This change explicitly detects missing Lake Formation grants and ensures the test fails when no tables are readable in the targeted schemas. ✅ 2 resolved✅ Edge Case: Mandatory GetTables now fails for legitimately empty/view-only catalogs
✅ Performance: Redundant get_schema_names() calls across executors
OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
Fixes #29160
Problem
The Athena connector's Test Connection reported all four steps green —
CheckAccess, GetSchemas, GetTables, GetViews — even when the IAM role was
missing the AWS Lake Formation
DESCRIBE/SELECTgrants needed to read thetarget tables. The metadata run then "succeeded" but ingested 0 tables,
giving users no signal that anything was wrong.
Why it passed
Formation silently filters results — when grants are missing it returns
an empty list instead of an error. pyathena reinforces this: its
list_table_metadata/_get_schemaseven convert someClientErrors into[]. Soinspector.get_table_names(schema)came back empty, the step wasmarked passed, and nothing raised.
all_schemas[0]),not the schemas the configured
schemaFilterPatternactually targets.Catalog vs. schema
catalogIdis the AWS data catalog (the level above schemas), injectedinto the connection URL as
&catalog_name=<catalogId>. pyathena scopeslist_databases/list_table_metadatato it, soget_schema_names()alreadyreturns the databases within the configured catalog and
get_table_names()is already catalog-scoped. A single Athena service maps to a single catalog
(Athena doesn't override
get_database_names), so detecting "zero readabletables across the targeted schemas" via the inspector is a faithful proxy for
the same condition that makes ingestion yield 0 tables.
Fix
GetTablesnow selects the schemasschemaFilterPatternwould target(reusing
filter_by_schema, capped atMAX_SCHEMAS_TO_PROBE = 100so acatalog with many databases can't exhaust the 3-minute timeout) and raises
when no tables are readable in any of them — with a message naming the catalog
and pointing the user at Lake Formation
DESCRIBE/SELECTgrants. It passesas soon as any one targeted schema is readable.
GetViewsuses the same schema selection but never raises on empty(views are legitimately often absent; the step is non-mandatory).
GetTableserrorMessagenow mentionsLake Formation so the user-visible one-liner is actionable.
Type of change:
High-level design:
N/A — small change.
Tests:
Use cases covered
Unit tests
Backend integration tests
Ingestion integration tests
Playwright (UI) tests
Manual testing performed
UI screen recording / screenshots:
Not applicable.
Checklist:
Fixes <issue-number>: <short explanation>Fixes #<issue-number>above.Greptile Summary
This PR fixes a silent false-positive in the Athena connector's Test Connection flow: AWS Lake Formation returns empty lists (not errors) when grants are missing, so the old executor reported all steps green while ingestion would produce 0 tables. The fix makes
GetTablesinspect the schemas the configuredschemaFilterPatternactually targets and raise a descriptiveRuntimeErrorwhen no tables are readable across all of them._get_targeted_schemas()mirrors the ingestion-time schema filter and caps probing atMAX_SCHEMAS_TO_PROBE = 100to avoid exhausting the 3-minute timeout on wide catalogs.custom_executor_for_tableshort-circuits as soon as any one targeted schema has at least one readable table;custom_executor_for_viewis probe-only and never raises, because views are legitimately absent.Confidence Score: 5/5
Safe to merge — the change is scoped entirely to the Athena test-connection path and does not touch ingestion runtime, schema listing logic, or any shared utilities.
The implementation correctly mirrors the ingestion-time schema filter, handles the Lake Formation silent-empty-list behaviour, caps probing to avoid timeouts, and memoises schema listing across the two executor steps. All new code paths are covered by targeted unit tests that verify both the raise and the pass conditions. No existing tests are broken and no shared code was modified.
No files require special attention.
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant TC as test_connection() participant GS as get_targeted_schemas() [memoised] participant Insp as SQLAlchemy Inspector participant LF as AWS Lake Formation (via pyathena) TC->>GS: get_targeted_schemas() GS->>Insp: get_schema_names() Insp->>LF: list_databases (catalog-scoped) LF-->>Insp: ["db1", "db2", ...] Insp-->>GS: schema list GS->>GS: filter_by_schema + cap at MAX_SCHEMAS_TO_PROBE GS-->>TC: targeted_schemas (cached) note over TC: GetTables step TC->>Insp: get_table_names(schema) for each targeted schema Insp->>LF: list_table_metadata (catalog-scoped) alt Lake Formation grants missing LF-->>Insp: [] (silently filtered) Insp-->>TC: [] TC-->>TC: raise RuntimeError (Lake Formation DESCRIBE/SELECT missing) else Grants present LF-->>Insp: ["t1", "t2"] Insp-->>TC: ["t1", "t2"] TC-->>TC: "any() == True → pass" end note over TC: GetViews step (non-mandatory, never raises) TC->>GS: get_targeted_schemas() [cache hit, no new call] GS-->>TC: targeted_schemas TC->>Insp: get_view_names(schema) for each targeted schema Insp-->>TC: views or [] TC-->>TC: break on first non-empty, or finish quietly%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant TC as test_connection() participant GS as get_targeted_schemas() [memoised] participant Insp as SQLAlchemy Inspector participant LF as AWS Lake Formation (via pyathena) TC->>GS: get_targeted_schemas() GS->>Insp: get_schema_names() Insp->>LF: list_databases (catalog-scoped) LF-->>Insp: ["db1", "db2", ...] Insp-->>GS: schema list GS->>GS: filter_by_schema + cap at MAX_SCHEMAS_TO_PROBE GS-->>TC: targeted_schemas (cached) note over TC: GetTables step TC->>Insp: get_table_names(schema) for each targeted schema Insp->>LF: list_table_metadata (catalog-scoped) alt Lake Formation grants missing LF-->>Insp: [] (silently filtered) Insp-->>TC: [] TC-->>TC: raise RuntimeError (Lake Formation DESCRIBE/SELECT missing) else Grants present LF-->>Insp: ["t1", "t2"] Insp-->>TC: ["t1", "t2"] TC-->>TC: "any() == True → pass" end note over TC: GetViews step (non-mandatory, never raises) TC->>GS: get_targeted_schemas() [cache hit, no new call] GS-->>TC: targeted_schemas TC->>Insp: get_view_names(schema) for each targeted schema Insp-->>TC: views or [] TC-->>TC: break on first non-empty, or finish quietlyReviews (2): Last reviewed commit: "refactor(ingestion): address review feed..." | Re-trigger Greptile