Skip to content

Fix process killing with subprocess API #30207

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
potiuk merged 2 commits into from
Mar 21, 2023
Merged

Fix process killing with subprocess API #30207

potiuk merged 2 commits into from
Mar 21, 2023

Conversation

@uranusjr
Copy link
Member

@uranusjr uranusjr commented Mar 21, 2023

#30188 broke airflow webserver the other way around. Both poll and is_running needs to be considered.

@boring-cyborg boring-cyborg bot added area:CLI area:webserver Webserver related Issues labels Mar 21, 2023
@uranusjr uranusjr force-pushed the fix-webserver-non-daemon branch from 1d06aba to d2b284f Compare March 21, 2023 08:59
potiuk
potiuk approved these changes Mar 21, 2023
View reviewed changes
Copy link
Member

@potiuk potiuk left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Interesting. Yeah I see now where it came from - I missed the with subprocess.Popen as gunicorn_master_proc in the non-daemon branch :(

Thanks for catching it.

@potiuk potiuk merged commit d9728eb into apache:main Mar 21, 2023
@potiuk potiuk added this to the Airflow 2.5.3 milestone Mar 21, 2023
@uranusjr uranusjr deleted the fix-webserver-non-daemon branch March 22, 2023 06:08
@pierrejeambrun pierrejeambrun added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Mar 22, 2023
pierrejeambrun pushed a commit that referenced this pull request Mar 24, 2023
@uranusjr @pierrejeambrun
Fix process killing with subprocess API (#30207)
e7946e8 
* Fix process killing with subprocess API

* Improve signal handler typing

(cherry picked from commit d9728eb)
@potiuk potiuk mentioned this pull request Mar 30, 2023
Closed
28 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@potiuk potiuk potiuk approved these changes

@pierrejeambrun pierrejeambrun Awaiting requested review from pierrejeambrun

Assignees

No one assigned

Labels

area:CLI area:webserver Webserver related Issues changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..)

Projects

None yet

Milestone

Airflow 2.5.3

Development

Successfully merging this pull request may close these issues.

3 participants

@uranusjr @potiuk @pierrejeambrun