techcodie commented on PR #63775:
URL: https://github.com/apache/airflow/pull/63775#issuecomment-4324990186
Addressed all the review feedback from @kaxil, @jscheffl, and the copilot
reviewer in `4b4f44362d`. Branch is also now rebased onto latest `main` (no
more conflicts).
### Changes in this push
**Hardened auth paths in `databricks_base.py`** — replaced `conn.password or
""` with explicit `ValueError` raises so a missing password fails loudly
instead of sending empty creds:
- `_a_get_sp_token` (line 340)
- `_a_get_aad_token` (line 441, AAD client_secret path)
- `_a_do_api_call` basic-auth fallback (line 1232)
**`_a_get_aad_token_for_default_az_credential`** (line 512) — removed the
discarded `await self.a_databricks_conn()` call. The method only uses
`self.oauth_tokens` and `DefaultAzureCredential`, so the connection fetch was a
wasted round-trip; cache-warming is `__aenter__`'s job.
**`_a_endpoint_url`** (line 1124) — added `-> str` return annotation and
typed `endpoint: str`.
**`_a_do_api_call` headers** (line 1260) — dropped the redundant
`{**headers, **self.user_agent_header}` re-merge. The earlier merge at line
1220 already includes `user_agent_header`, and the second merge silently
flipped precedence so user-agent overrode AAD headers — inconsistent with the
sync path. Now passes `headers=headers` so AAD headers win, matching
`_do_api_call`.
**`_a_do_api_call` comment** (line 1230) — rewrote the misleading
"cache-clash" comment to actually explain the motivation: avoiding the sync
`get_connection()` (AsyncToSync) path inside the triggerer event loop, with
`a_databricks_conn()` having already primed the `@cached_property` cache.
**`DatabricksSQLStatementExecutionTrigger.run`** (triggers/databricks.py) —
handled the immediate-timeout case (`end_time <= time.time()` at start)
explicitly before the polling loop, and removed the `statement_state =
SQLStatementState(state="PENDING")` pre-init. The loop-timeout path now emits
the actual last-polled state instead of a fabricated one.
**`utils/openlineage.py`** (line 27) — replaced `from airflow.utils import
timezone # type: ignore[attr-defined]` with `from
airflow.providers.common.compat.sdk import timezone`, matching the
cross-version pattern used elsewhere in the repo (`exceptions.py`,
`sensors/databricks.py`, etc.) and dropping the type-ignore.
All previously-resolved threads still apply; no unresolved review threads
remaining. Ready for re-review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
