Fix-16888: sas viya 4 table type error fixes

[harshsoni2024]

Describe your changes:

Fixes #16888

Type of change:

• Bug fix
• Improvement
• New feature
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation

Checklist:

• I have read the CONTRIBUTING document.
• My PR title is Fixes <issue-number>: <short explanation>
• I have commented on my code, particularly in hard-to-understand areas.
• For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

---

Summary by Gitar

• Null extension attribute handling:
  • Strip null values from table_extension before creating CreateTableRequest to prevent backend rejection
  • Added validation and comment explaining null rejection by custom-field types
• resourceId parsing robustness:
  • Added format validation in create_database_schema to catch malformed resourceId early
  • Expanded exception handling to catch IndexError, KeyError, ValueError alongside HTTPError
• Sink failure tolerance:
  • Early return when table entity is None after yield to skip failed patch/profile calls
  • Added logger warning to indicate sink-side rejection
• Exception handler safety:
  • Fixed undefined table_name reference in exception handler by deriving from table dict or fallback to id

_{This will update automatically on new commits.}

[Copilot]

Pull request overview

Fixes SAS Viya 4 ingestion failures caused by nullable custom attributes (e.g., casHost=None) and improves resiliency of SAS resourceId parsing/error handling so sink-side failures don’t get masked by source-side exceptions.

Changes:

• Filter None values out of SAS table extension attributes before emitting CreateTableRequest.
• Harden create_database_schema parsing and broaden exception handling to route malformed resourceId cases to the fallback path.
• Add unit tests covering the regression scenarios from #16888 (null extension, sink rejection follow-up, malformed resourceId, safer exception handler).

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/sas/metadata.py	Drops null custom attributes, avoids None.id masking errors after sink rejection, and improves resourceId parsing/fallback behavior.
ingestion/tests/unit/topology/database/test_sas.py	Adds regression-focused unit tests for #16888 across schema parsing, extension filtering, and exception-safety paths.

Comments suppressed due to low confidence (1)

ingestion/src/metadata/ingestion/source/database/sas/metadata.py:270

• In create_database_schema, the fallback except block assumes table["relationships"] exists. Since this method now catches KeyError/ValueError for malformed inputs, it can still crash with a new KeyError when relationships is missing (or None) and we enter the fallback. Please guard with table.get("relationships") (defaulting to []) and bail out cleanly (or log) when relationships are absent.

        except (HTTPError, IndexError, KeyError, ValueError) as _:
            # Find the "database" entity in Information Catalog
            # First see if the table is a member of the library through the relationships attribute
            # Or we could use views to query the dataStores
            data_store_data_sets = "4b114f6e-1c2a-4060-9184-6809a612f27b"
            data_store_id = None
            for relation in table["relationships"]:
                if relation["definitionId"] != data_store_data_sets:

[github-actions]

🔴 Playwright Results — 1 failure(s), 33 flaky

✅ 3647 passed · ❌ 1 failed · 🟡 33 flaky · ⏭️ 89 skipped

Shard	Passed	Failed	Flaky	Skipped
🔴 Shard 1	473	1	6	4
🟡 Shard 2	646	0	5	7
🟡 Shard 3	649	0	7	1
🟡 Shard 4	627	0	7	27
🟡 Shard 5	610	0	1	42
🟡 Shard 6	642	0	7	8

Genuine Failures (failed on all attempts)

❌ Pages/SearchIndexApplication.spec.ts › Search Index Application (shard 1)

Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoEqual�[2m(�[22m�[32mexpected�[39m�[2m) // deep equality�[22m

Expected: �[32mStringMatching /success|activeError/g�[39m
Received: �[31m"failed"�[39m

🟡 33 flaky test(s) (passed on retry)

• Features/DataAssetRulesDisabled.spec.ts › Verify the ApiEndpoint entity item action after rules disabled (shard 1, 2 retries)
• Features/DataAssetRulesDisabled.spec.ts › Verify the Chart entity item action after rules disabled (shard 1, 1 retry)
• Features/DataAssetRulesDisabled.spec.ts › Verify the Database entity item action after rules disabled (shard 1, 2 retries)
• Flow/Metric.spec.ts › Verify Related Metrics Update (shard 1, 1 retry)
• Pages/Customproperties-part1.spec.ts › Markdown (shard 1, 1 retry)
• Pages/UserCreationWithPersona.spec.ts › Create user with persona and verify on profile (shard 1, 1 retry)
• Features/BulkEditEntity.spec.ts › Glossary (shard 2, 1 retry)
• Features/BulkImport.spec.ts › Table (shard 2, 1 retry)
• Features/ChangeSummaryBadge.spec.ts › AI badge should NOT appear for manually-edited descriptions (shard 2, 1 retry)
• Features/DomainFilterQueryFilter.spec.ts › Domain page assets tab should show only domain assets (shard 2, 1 retry)
• Features/Glossary/GlossaryRemoveOperations.spec.ts › should add and remove reviewer from glossary term (shard 2, 1 retry)
• Features/RestoreEntityInheritedFields.spec.ts › Validate restore with Inherited domain and data products assigned (shard 3, 1 retry)
• Features/RestoreEntityInheritedFields.spec.ts › Validate restore with Inherited domain and data products assigned (shard 3, 1 retry)
• Features/RestoreEntityInheritedFields.spec.ts › Validate restore with Inherited domain and data products assigned (shard 3, 1 retry)
• Features/RTL.spec.ts › Verify Following widget functionality (shard 3, 1 retry)
• Features/UserProfileOnlineStatus.spec.ts › Should not show online status for inactive users (shard 3, 1 retry)
• Flow/CustomizeWidgets.spec.ts › Data Products Widget (shard 3, 1 retry)
• Flow/PersonaFlow.spec.ts › Set default persona for team should work properly (shard 3, 1 retry)
• Pages/Customproperties-part2.spec.ts › entityReferenceList shows item count, scrollable list, no expand toggle (shard 4, 1 retry)
• Pages/DataContracts.spec.ts › Create Data Contract and validate for Table (shard 4, 1 retry)
• Pages/DataContracts.spec.ts › Create Data Contract and validate for Directory (shard 4, 1 retry)
• Pages/Domains.spec.ts › Multiple consecutive domain renames preserve all associations (shard 4, 1 retry)
• Pages/DomainUIInteractions.spec.ts › Add owner to data product via UI (shard 4, 1 retry)
• Pages/Entity.spec.ts › Glossary Term Add, Update and Remove (shard 4, 1 retry)
• Pages/Entity.spec.ts › User as Owner with unsorted list (shard 4, 1 retry)
• Pages/Glossary.spec.ts › Add and Remove Assets (shard 5, 1 retry)
• Pages/Lineage/DataAssetLineage.spec.ts › verify create lineage for entity - Table (shard 6, 1 retry)
• Pages/Lineage/LineageFilters.spec.ts › Verify lineage schema filter selection (shard 6, 1 retry)
• Pages/Lineage/LineageRightPanel.spec.ts › Verify custom properties tab IS visible for supported type: searchIndex (shard 6, 1 retry)
• Pages/Login.spec.ts › Refresh should work (shard 6, 1 retry)
• ... and 3 more

📦 Download artifacts

How to debug locally

# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

[gitar-bot]

Code Review ✅ Approved 1 resolved / 1 findings

Removes the redundant ValueError in the SAS Viya 4 table type validation logic, allowing existing IndexError handling to suffice. No issues were found.

✅ 1 resolved

✅ Quality: ValueError raise is redundant; IndexError would be caught anyway

📄 ingestion/src/metadata/ingestion/source/database/sas/metadata.py:240-244 📄 ingestion/src/metadata/ingestion/source/database/sas/metadata.py:263
Lines 240-244 explicitly raise a ValueError when context_parts has fewer than 5 elements. However, the subsequent indexing on lines 247-248 (context_parts[2], context_parts[4]) would naturally raise an IndexError, which is already caught by the same except clause on line 263. The explicit check adds a better error message (good), but consider that the IndexError catch on line 263 was presumably added for this exact scenario — making the explicit raise partially redundant. This is minor and the explicit check does improve debuggability, so it's fine to keep if intentional.

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact

gitar display:verbose

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

[sonarqubecloud]

Quality Gate passed for 'open-metadata-ingestion'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
90.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud