Skip to content
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.

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

Ensure __exit__ is called in decorator context managers #38383

Merged
merged 2 commits into from
Mar 21, 2024

Conversation

dstandish
Copy link
Contributor

@dstandish dstandish commented Mar 21, 2024

In #36800 author fixed zombie scheduler issue arising from context manager exit not being called, thus sub process not getting terminated. It was fixed by explicitly calling the close function on an ExitStack-managed context manager. Simpler / better / cleaner / more standard solution is to "fix" the underlying context managers by wrapping the yield in a try / finally.

cc @joaopamaral

In apache#36800 author fixed zombie scheduler issue arising from context manager exit not being called, thus sub process not getting terminated.  It was fixed by explicitly calling the `close` function on an ExitStack-managed context manager.  Simpler / better / cleaner / more standard solution is to "fix" the underlying context managers by wrapping the yield in a try / finally.
@dstandish dstandish merged commit 095c5fe into apache:main Mar 21, 2024
47 checks passed
@dstandish dstandish deleted the fix-context-managers branch March 21, 2024 21:06
@ephraimbuddy ephraimbuddy added the type:bug-fix Changelog: Bug Fixes label Mar 23, 2024
utkarsharma2 pushed a commit to astronomer/airflow that referenced this pull request Apr 22, 2024
In apache#36800 author fixed zombie scheduler issue arising from context manager exit not being called, thus sub process not getting terminated.  It was fixed by explicitly calling the `close` function on an ExitStack-managed context manager.  Simpler / better / cleaner / more standard solution is to "fix" the underlying context managers by wrapping the yield in a try / finally.
@ephraimbuddy ephraimbuddy added this to the Airflow 2.9.2 milestone Jun 3, 2024
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.

5 participants