potiuk commented on code in PR #70528:
URL: https://github.com/apache/airflow/pull/70528#discussion_r3681905352
##########
providers/google/src/airflow/providers/google/cloud/sensors/bigquery_dts.py:
##########
@@ -144,4 +144,4 @@ def poke(self, context: Context) -> bool:
if run.state in (TransferState.FAILED, TransferState.CANCELLED):
message = f"Transfer {self.run_id} did not succeed"
raise AirflowException(message)
- return run.state in self.expected_statuses
+ return run.state in self._normalize_state_list(self.expected_statuses)
Review Comment:
Two small consequences of moving the normalisation here, neither blocking:
**Error quality.** `_normalize_state_list` does
`TransferState[state.upper()]`, so a typo'd status now surfaces as a bare
`KeyError: 'SUCEEDED'` from inside the sensor on every poke, rather than at
parse time where the traceback at least pointed at the Dag file. That was
survivable when it failed fast at parse; as a runtime failure it's worth
wrapping in a `ValueError` naming the offending value and the valid ones.
Arguably out of scope here, but this PR is what makes it user-visible.
**Repeated work.** Normalisation now runs on every poke rather than once.
It's cheap and a sensor is I/O-bound, so this is genuinely negligible —
mentioning it only so it's a conscious choice rather than an oversight. If
you'd rather avoid it, normalising once into a cached attribute on first poke
would do it.
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
--
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]
