You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/12/10 16:52:33 UTC
[GitHub] [airflow] potiuk opened a new pull request, #28283: Trigger gevent monkeypatching via environment variable
potiuk opened a new pull request, #28283:
URL: https://github.com/apache/airflow/pull/28283
Gevent needs to monkeypatch a number of system libraries as soon as possible when Python interpreter starts, in order to avoid other libraries monkey-patching them before. We should do it before any other initialization and it needs to be only run on webserver.
So far it was done by local_settings monkeypatching but that has been rather brittle and some changes in Airflow made previous attempts to stop working because the "other" packages could be loaded by Airflow before - depending on installed providers and configuration (for example when you had AWS configured as logger, boto could have been loaded before and it could have monkey patch networking before gevent had a chance to do so.
This change introduces different mechanism of triggering the patching - it could be triggered by setting an environment variable. This has the benefit that we do not need to initialize anything (including reading settings or setting up logging) before we determine if gevent patching should be performed.
It has also the drawback that the user will have to set the environment variable in their deployment manually. However this is a small price to pay if they will get a stable and future-proof gevent monkeypatching built-in in Airflow.
Fixes: #8212
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jedcunningham commented on a diff in pull request #28283: Trigger gevent monkeypatching via environment variable
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on code in PR #28283:
URL: https://github.com/apache/airflow/pull/28283#discussion_r1052456738
##########
newsfragments/08212.misc.rst:
##########
@@ -0,0 +1 @@
+If you are using gevent for your webserver deployment and used local settigns to monkeypatch gevent, you might want to replace local settings patching with setting the ``_AIRFLOW_PATCH_GEVENT`` variable to 1 in your webserver.
Review Comment:
```suggestion
If you are using gevent for your webserver deployment and used local settings to monkeypatch gevent, you might want to replace local settings patching with an ``_AIRFLOW_PATCH_GEVENT`` environment variable set to 1 in your webserver. This ensures gevent patching is done as early as possible.
```
##########
airflow/config_templates/config.yml:
##########
@@ -1222,7 +1222,9 @@
- name: worker_class
description: |
The worker class gunicorn should use. Choices include
- sync (default), eventlet, gevent
+ sync (default), eventlet, gevent. Note that in case of gevent you might also want to set
Review Comment:
```suggestion
sync (default), eventlet, gevent. Note when using gevent you might also want to set the
```
nit, just sounds better to my ear
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk merged pull request #28283: Trigger gevent monkeypatching via environment variable
Posted by GitBox <gi...@apache.org>.
potiuk merged PR #28283:
URL: https://github.com/apache/airflow/pull/28283
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28283: Trigger gevent monkeypatching via environment variable
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #28283:
URL: https://github.com/apache/airflow/pull/28283#issuecomment-1356898026
Seems we have confirmation that at least this part of the gevent configuration (monkeypatching) works with this approach.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28283: Trigger gevent monkeypatching via environment variable
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #28283:
URL: https://github.com/apache/airflow/pull/28283#issuecomment-1353623910
Waiting for @victorjourne to test this one, but I believe it should fix the gevent problem, so kind request for review
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] victorjourne commented on pull request #28283: Trigger gevent monkeypatching via environment variable
Posted by GitBox <gi...@apache.org>.
victorjourne commented on PR #28283:
URL: https://github.com/apache/airflow/pull/28283#issuecomment-1356892685
I can confirm that this fix solves my issue #28380 and #8164 warnings have disappeared.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org