kaxil commented on PR #73142:
URL: https://github.com/apache/airflow/pull/73142#issuecomment-5673346303
One non-blocking follow-up to my inline comments, on
`airflow-core/newsfragments/73142.bugfix.rst`.
The fragment carries the same mechanism as the docstring, so it inherits the
same problem: on this path the exit code isn't unobserved.
`_monitor_subprocess()` loops on `while self._exit_code is None or
self._open_sockets:` and its only `break` is nested inside `if self._exit_code
is not None`, so it can't return with the code still unset, and `wait()`'s
`else 1` default can't be what overrode the terminal state here.
If you keep a fragment, describing the user-visible symptom instead of the
mechanism stays accurate whichever root cause lands: something like "a task
that had already reported success could still be marked `up_for_retry`, failing
the worker with a 409".
Dropping it is also fine by me. Newsfragments are optional ("Consider adding
a newsfragment", `contributing-docs/18_contribution_workflow.rst`), and for a
routine bugfix the PR and the closed issue are already the record.
--
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]
