namanjain24-sudo commented on PR #75423:
URL: https://github.com/apache/airflow/pull/75423#issuecomment-6358759465
Thanks — you're right on all three, and the first is a real bug, not a nit.
I reproduced it before changing anything:
```
task type : SerializedBaseOperator
dag type : SerializedDAG
get_template_env() -> AttributeError: 'SerializedDAG' object has no
attribute 'get_template_env'
```
So the branch never got past that line: the `AttributeError` hit my own
`except`, every start-from-trigger
task logged "Failed to render template fields", and nothing was sent. The
feature was a no-op. The tests
passed only because they handed `model_construct` an SDK `BaseOperator`,
which is not what
`create_runtime_ti` builds — exactly the gap you point at.
I also confirmed the history: #53071 loaded the Dag in the triggerer and was
reverted in #55037, so adding
the missing methods to the serialized classes is not a way out either.
Rewritten to record only what the triggerer can actually render.
`BaseTrigger.template_fields` is already
narrowed to the operator fields that `start_trigger_args.trigger_kwargs`
carries, so
`record_rendered_fields` now runs straight after
`trigger.render_template_fields(context)` and serializes
the trigger. No Dag file, no serialized-operator rendering. That covers the
issue's case (`target_time` on
`DateTimeSensorAsync`) and only that class of field, which the docstring now
says plainly.
On the tests, I replaced the hand-built fixture with one that drives the
real path: `create_trigger_in_db`
with a `start_from_trigger` sensor, then `supervisor.load_triggers()` →
`sync_state_to_supervisor` →
`create_triggers()` → the trigger's own task, asserting the recorded value
is the rendered one. Against the
old code that test fails with precisely the production error:
```
[error] Failed to render template fields
AttributeError: 'SerializedDAG' object has no attribute 'get_template_env'
assert [] == [{'target_time': '2023-01-01 00:00:00+00:00'}]
```
I also checked the id the fields are recorded against: `_current_trigger_ti`
holds the right task instance
at that point, which is what `TriggerCommsDecoder.asend` stamps on the frame
(already covered by
`test_asend_frame_carries_the_current_trigger_ti_id`), and the supervisor
reads it back into
`_handling_trigger_ti_id`. Added the missing test for the
`SetRenderedFields` branch in `_handle_request`
asserting both the `set_rtif` call and the `OKResponse`.
Newsfragment dropped — thanks for the pointer to
`18_contribution_workflow.rst`.
Full `test_triggerer_job.py` is green (138 passed, 1 skipped) and mypy is
clean on the changed file.
---
Drafted-by: Claude Code (Opus 5); reviewed by @namanjain24-sudo 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]
