Repository navigation
Document that clear_on_success also removes the KPO pod identity - #71749
Conversation
The durable execution section says the persisted pod identity is only removed by `airflow state-store clean`, but `[state_store] clear_on_success` clears the whole task scope as soon as the task instance succeeds. It is off by default, so the sentence holds for most deployments but not for the ones that turn it on. Generated-by: Claude Code following https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Review nits: reflow the paragraph so the retry_delay sentence no longer leaves an orphan line, and point readers at the core task state store docs for the setting. Generated-by: Claude Opus 5
941d87a to
ac18eea
Compare
potiuk
left a comment
There was a problem hiding this comment.
Verified against main: clear_on_success really does clear the whole task scope on success in the execution API (execution_api/routes/task_instances.py), on the public mark-success path, and on custom backends in the task runner, so the old "only airflow state-store clean" sentence was wrong for deployments that enable it, and this correction is accurate.
Also confirmed the retained sentences about default_retention_days / retry_delay still hold: MetastoreBackend.get() does not filter on expires_at, so state-store clean remains the only retention-driven deletion. Nice catch, and thanks for keeping the PR scoped to the one sentence.
I pushed a small fixup on top: the paragraph is reflowed (the retry_delay sentence had ended up as a short orphan line) and the sentence now links the core task state store docs for the setting. I also rebased the branch on main so CI runs against current code.
This review was drafted by an AI-assisted tool and
confirmed by an Apache Airflow maintainer. The maintainer
approving this PR has read the findings and signed off. If
something feels off, please reply on the PR and a maintainer
will follow up.More on how Apache Airflow handles maintainer review:
contributing-docs/05_pull_requests.rst.
|
Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions. |
What is wrong
The durable execution section of the KubernetesPodOperator doc say:
But
[state_store] clear_on_successdelete it too. When a task instance reach SUCCESS, the execution API clear the whole task scope from the state store:The default is
False, so most people never see it. But someone who turn it on read this sentence and think the pod identity still stay, and then the next retry go to the label search instead of the direct reconnect, and they do not know why.What I change
Only that one sentence. The rest of the paragraph, the part about
retry_delayanddefault_retention_days, I do not touch.I find this while working on #71743, sorry for the small PR. Thank you for reading it.
Was generative AI tooling used to co-author this PR?
This PR was written in part with the assistance of generative AI. I have reviewed and tested every change myself.