fix(data-quality): pass Azure SQL connection parameters to data diff - #31057
fix(data-quality): pass Azure SQL connection parameters to data diff#31057IceS2 wants to merge 5 commits into
Conversation
Azure SQL builds its connection URL with an ODBC connection string, which keeps the credentials inside the odbc_connect query parameter where data-diff cannot read them. Hand it an explicit connection dict instead, carrying the per-table database and schema. Also drop the Snowflake password from the data-diff URL when a private key is configured, since data-diff rejects both at once. Fixes #31044
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
github-actions
Bot
added
Ingestion
safe to test
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| from metadata.ingestion.ometa.ometa_api import OpenMetadata | ||
| from metadata.utils.constants import THREE_MIN | ||
|
|
||
| DEFAULT_SQL_SERVER_PORT = 1433 |
There was a problem hiding this comment.
I don't think we would need that right. The connection should fail earlier in the code if the service_connection port section is null right?
There was a problem hiding this comment.
From what I tested, it actually could accept None and technically we could pass it and it would strip it and works (It only seems to break if we don't pass the port as it receives it as positional arg)
I like the explicit nature here and think about this more as defense in depth
URL.set(password=None) leaves the password untouched, so rebuild the url without it and assert the result through the parser data-diff itself uses. Also resolve the connection scheme through its default so the driver name is always set.
✅ Playwright Results — workflow succeededValidated commit ✅ 107 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky PerformanceBlocking targets: ✅ met · Optimization targets: 🟡 in progress Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting. 🕒 Full workflow signal wall (to summary) 55m 48s ⏱️ Max setup 3m 5s · max shard execution 10m 27s · max shard-job elapsed before upload 18m 39s · reporting 3s 🌐 208.70 requests/attempt · 1.76 app boots/UI scenario · 0.00% common-shard skew Optimization targets still in progress:
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
Code Review ✅ Approved 1 resolved / 1 findingsPasses Azure SQL connection parameters as an explicit dictionary and drops Snowflake passwords when using private keys, addressing the Snowflake password cleared with empty string, not removed finding. ✅ 1 resolved✅ Bug: Snowflake password cleared with empty string, not removed
OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar | Powered by Gitar — free for open source |
|
🚦 Removed from the merge queue —
|
Fixes #31044
Azure SQL builds its connection URL with an ODBC connection string, so
data-diff cannot read host, port, user and password from it and fails to
construct the MsSQL client.
The connection is now handed to data-diff as an explicit connection dict —
the same path Trino and Oracle already use — with the per-table database and
schema applied.
Also drops the Snowflake password from the data-diff URL when a private key is
configured, since data-diff rejects receiving both.