Repository navigation
Normalize BigQuery DTS sensor expected statuses after rendering - #70528
Conversation
boring-cyborg
Bot
added
area:dev-tools
area:providers
backport-to-v3-3-test
provider:google
potiuk
added
the
ready for maintainer review
potiuk
left a comment
potiuk
left a comment
There was a problem hiding this comment.
Thanks — correct, and this one is the clearest case of the pattern. expected_statuses is in template_fields, so normalising it in __init__ ran against the raw "{{ ... }}" string: _normalize_state_list would try TransferState["{{ VAR.VALUE.EXPECTED_STATUS }}"] and blow up at parse time before the value ever existed. Deferring to poke is the only way this can work.
A side benefit worth noting: self.expected_statuses now holds the rendered value rather than a set[TransferState], which makes the rendered-template view in the UI show what the user actually wrote instead of an enum set.
One observation inline about what a bad value now looks like.
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
8637f8f to
dc4e9b5
Compare
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions. |
github-actions
Bot
added
the
stale
Rebased onto current main. Validate expected_statuses after template rendering so templated values are checked at runtime.
shahar1
removed
the
stale
Since a prior change moved expected_statuses normalization from __init__ into poke() (expected_statuses is a template field, so it has to be normalized against its rendered value rather than the raw Jinja string), a misspelled status name no longer fails at Dag parse time. Instead, the TransferState[state.upper()] lookup raises a bare KeyError from inside poke(). A bare KeyError carries nothing but the key. BaseSensorOperator in task-sdk wraps every poke() call in a broad except Exception (task-sdk/src/airflow/sdk/bases/sensor.py), and that arm is type-agnostic, so the control flow is the same either way: with silent_fail=True the error is logged and poke_return becomes False, with never_fail=True it is chained into an AirflowSkipException, and otherwise it propagates and fails the task. What changes is what the user reads in each of those three places. Catch the KeyError where the string lookup happens and re-raise it as a ValueError that names the offending value and lists the valid TransferState names, so the failure is immediately actionable wherever it surfaces.
The templated expected_statuses test hand-assigned the rendered value instead of exercising Airflow's template rendering machinery, so it never actually proved that expected_statuses renders correctly at poke time. It now asserts the raw Jinja string survives construction and calls render_template_fields() to render it for real before poking. Also mark run_id, transfer_config_id, expected_statuses, and project_id as (templated) in the class docstring, since they are all listed in template_fields but only impersonation_chain was marked.
dc4e9b5 to
218c520
Compare
Backport failed to create: v3-3-test. View the failure log Run detailsNote: As of Merging PRs targeted for Airflow 3.X In matter of doubt please ask in #release-management Slack channel.
You can attempt to backport this manually by running: cherry_picker 9a1273c v3-3-testThis should apply the commit to the v3-3-test branch and leave the commit in conflict state marking After you have resolved the conflicts, you can continue the backport process by running: cherry_picker --continueIf you don't have cherry-picker installed, see the installation guide. |
Part of the template-field validation burn-down tracked in #70296.
BigQueryDataTransferServiceTransferRunSensorlistsexpected_statusesintemplate_fieldsbut converts it toTransferStatemembers in__init__via_normalize_state_list, so passing a Jinja expression raisedKeyErrorat parse time. The sensor now stores the raw value and normalizes it inpoke, after rendering.Added a test constructing the sensor with a templated
expected_statusesthat passes once the field holds the rendered value — it fails against the previous implementation. The class is removed from the exemption list and thevalidate-operators-initcheck passes locally.Per the discussion in #70505 this is a genuine value transformation (not an argument-provision check), so it belongs at execute time.
Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Fable 5) following the guidelines