Let a watcher trigger keep state when several assets watch it - #71460
Let a watcher trigger keep state when several assets watch it#714601fanwang wants to merge 1 commit into
Conversation
b9dfd15 to
d697cf4
Compare
d697cf4 to
29c7518
Compare
The accessors only expose the single-accessor shorthand, which raises when the caller serves more than one asset. A task knows its own inlets, but a caller that is handed the accessors does not, and the triggerer hands them to every BaseEventTrigger that has asset watchers. Triggers are deduplicated by hash(classpath, kwargs) while asset_watcher is many-to-many, so a trigger watched by two assets receives two accessors. Detecting that today means catching the ValueError, which a state store backend can also raise, so the catch can hide a real failure. Add __len__ to ask directly and __iter__ so such a caller can address every asset in turn rather than giving up on per asset state. Signed-off-by: 1fanwang <1fannnw@gmail.com>
29c7518 to
87c7cc1
Compare
|
Hey @1fanwang, sorry but I have second thoughts on this one. Could you explain in human english the change here? Sorry but I find it difficult recently to read these AI generated description. Can you really explain what this change does? What does it allow? A before your PR vs after your PR would help |
hey @vincbeck my bad - we do NOT need this PR, and thanks for raising this! I tried to walk through this again and repro, it proved me wrong and this only breaks in a case that doesn't really come up in practice: a single DAG waiting on two assets whose watchers are configured in the exact same way - declared in the same DAG file Airflow dedupes triggers within a single file parse, so both assets land on one trigger row, and the trigger gets two accessors: The watcher example in our docs assumes one accessor, so it raises and the triggerer restarts it in a loop: Split across two DAG files it doesn't happen at all. Closing this and #71751. |
Rationale for this change
Two teams watch the same
ordersfeed. Their triggers are identical, so Airflow keeps one trigger row serving both, on purpose: two pollers would hammer the source. It gets one accessor per asset, but the watcher example in our docs assumes only one:It raises
ValueError: Task has 2 concrete inlets and outlets — use context['asset_state_store'][MY_ASSET] to specify which, and neither team's pipeline runs. A trigger cannot take that advice: it has nocontext, and never learns its assets, since it is rebuilt fromserialize()kwargs that name none.What changes are included in this PR?
len()and iteration onAssetStateStoreAccessors, so a trigger can ask how many assets it serves and reach each.Are these changes tested?
Live, on Postgres with api-server, dag-processor, scheduler and triggerer as separate processes. Two assets watch the docs'
PollEventsTriggerunmodified, polling a file; both land on one trigger row.Before and after
Six unit tests cover
lenand iteration; all fail onmain.Are there any user-facing changes?
One asset behaves as before; several can now keep state at all.
get/set/delete/clearstill raise for several.#71354's
aget/aset/adelete/aclearhit the same_single_accessorand raise here too; the two compose. First consumer: #71751.