Skip to content

Add the resume event to stored next_kwargs without decoding them - #73576

Merged
potiuk merged 2 commits into
apache:mainfrom
potiuk:add-resume-event-without-decoding-next-kwargs
Oct 19, 2026
Merged

potiuk merged 2 commits into
apache:mainfrom
potiuk:add-resume-event-without-decoding-next-kwargs

Conversation

@potiuk

@potiuk potiuk commented Sep 22, 2026

Copy link
Copy Markdown
Member

handle_event_submit only needs to add the trigger event to a task instance's stored
next_kwargs. It previously decoded the whole stored payload, added the event, and
re-encoded everything — which reconstructs every stored value in the scheduler,
triggerer and API server, although only the worker ever needs those objects.

This merges the serialized event into the stored serialized dict instead. Stored values
pass through untouched (the worker deserializes them as before); payloads that are not
a dict at the top level still fail the task instance rather than resuming it. Rows still
in the legacy {"__type": "dict", "__var": ...} shape keep that shape.

Test plan:

  • Stored kwargs carrying a nested serialized object are preserved verbatim and the
    object is never constructed during event submission
  • Legacy-format stored kwargs get the event added in the same format
  • Existing unusable-kwargs and unserializable-payload cases still fail the task
  • airflow-core/tests/unit/models/test_trigger.py passes
Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 5.5)

Generated-by: Claude Code (Opus 5.5) following the guidelines at
https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions

handle_event_submit only needs to add the trigger event to a task
instance's stored next_kwargs. It previously decoded the whole stored
payload, added the event and re-encoded everything, reconstructing every
stored value in the scheduler, triggerer and API server although only the
worker ever needs those objects.

Merge the serialized event into the stored serialized dict instead. Stored
values pass through untouched; payloads that are not a dict at the top
level still fail the task instance rather than resuming it. Rows still in
the legacy {"__type": "dict", "__var": ...} shape keep that shape.

Generated-by: Claude Opus 5
@potiuk
potiuk requested review from XD-DENG and ashb as code owners September 22, 2026 21:53
@potiuk potiuk added this to the Airflow 3.4.0 milestone Sep 22, 2026
@potiuk

potiuk commented Sep 27, 2026

Copy link
Copy Markdown
Member Author

@ashb @amoghrajesh — gentle reminder: this needs a review to make the 3.4.0 cut on 2026-10-19. CI is green and it's ready from my side.

@potiuk

potiuk commented Oct 16, 2026

Copy link
Copy Markdown
Member Author

@uranusjr @amoghrajesh Could you take a look at this when you have a moment? It's targeted at 3.4.0 and needs a review to make the cut on 2026-10-19. CI is green.

@vatsrahul1001 vatsrahul1001 added the backport-to-v3-4-test Backport to v3-4-test label Oct 18, 2026

@vatsrahul1001 vatsrahul1001 left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Stored values are carried over encoded and the legacy form keeps its encoding. The worker already falls back to BaseSerialization.deserialize for legacy rows, and the tests check that nothing is constructed during event submission. Could use reviews from @uranusjr @amoghrajesh before merging.

@potiuk
potiuk merged commit 1117472 into apache:main Oct 19, 2026
79 checks passed
@potiuk
potiuk deleted the add-resume-event-without-decoding-next-kwargs branch October 19, 2026 14:56
@github-actions

Copy link
Copy Markdown
Contributor

Backport successfully created: v3-4-test

Note: As of Merging PRs targeted for Airflow 3.X
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.

In matter of doubt please ask in #release-management Slack channel.

Status Branch Result
✅ v3-4-test PR Link

potiuk added a commit that referenced this pull request Oct 19, 2026
…ng them (#73576) (#75116)

* Add the resume event to stored next_kwargs without decoding them

handle_event_submit only needs to add the trigger event to a task
instance's stored next_kwargs. It previously decoded the whole stored
payload, added the event and re-encoded everything, reconstructing every
stored value in the scheduler, triggerer and API server although only the
worker ever needs those objects.

Merge the serialized event into the stored serialized dict instead. Stored
values pass through untouched; payloads that are not a dict at the top
level still fail the task instance rather than resuming it. Rows still in
the legacy {"__type": "dict", "__var": ...} shape keep that shape.

Generated-by: Claude Opus 5

* Fail the task when a legacy resume event cannot be serialized
(cherry picked from commit 1117472)


Generated-by: Claude Opus 5

Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
vatsrahul1001 pushed a commit that referenced this pull request Oct 20, 2026
…ng them (#73576) (#75116)

* Add the resume event to stored next_kwargs without decoding them

handle_event_submit only needs to add the trigger event to a task
instance's stored next_kwargs. It previously decoded the whole stored
payload, added the event and re-encoded everything, reconstructing every
stored value in the scheduler, triggerer and API server although only the
worker ever needs those objects.

Merge the serialized event into the stored serialized dict instead. Stored
values pass through untouched; payloads that are not a dict at the top
level still fail the task instance rather than resuming it. Rows still in
the legacy {"__type": "dict", "__var": ...} shape keep that shape.

Generated-by: Claude Opus 5

* Fail the task when a legacy resume event cannot be serialized
(cherry picked from commit 1117472)


Generated-by: Claude Opus 5

Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants