Skip to content

Correctly ensure that we give subprocesses time to exit after signalling it#44766

Merged
ashb merged 1 commit intomainfrom
task-sdk-kill-escalation-tests
Dec 7, 2024
Merged

Correctly ensure that we give subprocesses time to exit after signalling it#44766
ashb merged 1 commit intomainfrom
task-sdk-kill-escalation-tests

Conversation

@ashb
Copy link
Member

@ashb ashb commented Dec 7, 2024

We had a bug hidden in our tests by our use of mocks -- if the subprocess
returned any output, then self.selector.select() would return straight away,
not waiting for the maximum timeout, which would result in the "escalation"
signal being sent after one outout, not after the given interval.

Discovered while looking into #44760 (comment)

…ing it

We had a bug hidden in our tests by our use of mocks -- if the subprocess
returned any output, then `self.selector.select()` would return straight away,
not waiting for the maximum timeout, which would result in the "escalation"
signal being sent after one outout, not after the given interval.
Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

NIce.

@ashb ashb merged commit 16206ef into main Dec 7, 2024
@ashb ashb deleted the task-sdk-kill-escalation-tests branch December 7, 2024 16:41
got686-yandex pushed a commit to got686-yandex/airflow that referenced this pull request Jan 30, 2025
…ing it (apache#44766)

We had a bug hidden in our tests by our use of mocks -- if the subprocess
returned any output, then `self.selector.select()` would return straight away,
not waiting for the maximum timeout, which would result in the "escalation"
signal being sent after one output, not after the given interval.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants