You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "Lorenzo-N (via GitHub)" <gi...@apache.org> on 2023/04/27 13:46:50 UTC
[GitHub] [airflow] Lorenzo-N opened a new issue, #30910: Reload_on_plugin_change does not work properly
Lorenzo-N opened a new issue, #30910:
URL: https://github.com/apache/airflow/issues/30910
### Apache Airflow version
2.5.3
### What happened
Hi, I updated airflow from version 2.2.0 to version 2.5.3.
I developed an airflow plugin that adds some REST API by implementing a flask_blueprint and in version 2.2.0 I could develop the plugin using the environment parameter **reload_on_plugin_change=True**. Updating to v2.5.3 this parameter seems to have no effect anymore, and if I change the plugin the changes are not updated until I restart the webserver.
### What you think should happen instead
I expect plugins to be reloaded upon change if the parameter **reload_on_plugin_change** is set to true.
### How to reproduce
- Set in the env the parameter **reload_on_plugin_change=True**.
- Develop a plugin that implements an api (I also tried the example plugin described here in the documentation [https://airflow.apache.org/docs/apache-airflow/2.5.3/authoring-and-scheduling/plugins.html#example](https://airflow.apache.org/docs/apache-airflow/2.5.3/authoring-and-scheduling/plugins.html#example)).
- Edit the plugin and reload the interface. The plugin is not updated.
### Operating System
Extension of official Docker image apache/airflow:2.5.3-python3.10
### Versions of Apache Airflow Providers
_No response_
### Deployment
Docker-Compose
### Deployment details
_No response_
### Anything else
I investigated the cause of the problem and it seems to be due to the [**--preload**](https://github.com/apache/airflow/blob/main/airflow/cli/commands/webserver_command.py#L427) parameter that is added to the gunicorn args. In fact, the workers are restarted when one of the plugin files changes, but the plugins are not updated (in my opinion because the parameter shares the app among all the workers so the plugins are not reloaded). Launching the webserver with the **--debug** parameter correctly reloads the plugins, however, I would like to use the gunicorn in development to test its correct operation as well.
In my opinion a solution might be not to pass the **--preload** parameter to the gunicorn if the variable **reload_on_plugin_change** is set, so that the plugins are reloaded correctly.
### Are you willing to submit PR?
- [ ] Yes I am willing to submit a PR!
### Code of Conduct
- [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md)
--
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.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #30910: Make "--preload" flag optional
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on issue #30910:
URL: https://github.com/apache/airflow/issues/30910#issuecomment-1531142790
Yes. The `--preload` flag has been added to optimize airlfow loading https://github.com/apache/airflow/pull/27297 and yes, it might break some workflows regarding the development of flask applications like you do.
We are targetting airflow for production use and this is clearly a development feature, so we do not classify it as a bug.
For now I recommend you to just comment this change out while you are developing the API. However i also encourage you (and anyone who my stumble upon this issue) to add a new feature where the `--preload` flag might be disabled for development case.. A good idea might be to not add the `--preload` flag when certain variable is set (we do not want to clutter airflow config with developer options) and add some documenttion in Plugin section of our docs about it.
I'd encourage you also to submit a PR fixing that @Lorenzo-N if you feel like contributing back to airflow is a good idea - otherwise it will wait for someone who will pick it up (I marked it as "good first issue").
--
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] Lorenzo-N commented on issue #30910: Make "--preload" flag optional
Posted by "Lorenzo-N (via GitHub)" <gi...@apache.org>.
Lorenzo-N commented on issue #30910:
URL: https://github.com/apache/airflow/issues/30910#issuecomment-1531293512
Could a simple change like https://github.com/apache/airflow/pull/31012 suffice, reusing the reload_on_plugin_change variable to disable the --preload flag?
--
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] kobethuwis commented on issue #30910: Reload_on_plugin_change does not work properly
Posted by "kobethuwis (via GitHub)" <gi...@apache.org>.
kobethuwis commented on issue #30910:
URL: https://github.com/apache/airflow/issues/30910#issuecomment-1527205522
I've experienced issues after upgrading my Airflow version as well a while ago. I managed to solve it by using a combination of two changes: https://github.com/apache/airflow/discussions/30281#discussioncomment-5647903
--
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] Lorenzo-N commented on issue #30910: Reload_on_plugin_change does not work properly
Posted by "Lorenzo-N (via GitHub)" <gi...@apache.org>.
Lorenzo-N commented on issue #30910:
URL: https://github.com/apache/airflow/issues/30910#issuecomment-1531072484
Thanks for the reply, but I think I have a different problem since I don't use Kubernetes, I already set reload_on_plugin_change=True in the env and I have the \_\_init__.py file in each plugin folder. Also I have the problem when I change any plugin file with api definitions, not a static file.
--
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 issue #30910: Make "--preload" flag optional
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on issue #30910:
URL: https://github.com/apache/airflow/issues/30910#issuecomment-1531320150
I believe it's not good . Reload on plugin change is a production setting. that allows you to reload generally plugins (but not flask API lie is your case). But I am not 100% sure - of that - someone who is more of an expert there shoud asses the impact of that.
Maybe you can asses that?
--
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] dejadunlap commented on issue #30910: Make "--preload" flag optional
Posted by "dejadunlap (via GitHub)" <gi...@apache.org>.
dejadunlap commented on issue #30910:
URL: https://github.com/apache/airflow/issues/30910#issuecomment-1645759490
Hi, if no one else has taken it can I take on this issue?
--
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] ephraimbuddy closed issue #30910: Make "--preload" flag optional
Posted by "ephraimbuddy (via GitHub)" <gi...@apache.org>.
ephraimbuddy closed issue #30910: Make "--preload" flag optional
URL: https://github.com/apache/airflow/issues/30910
--
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 issue #30910: Make "--preload" flag optional
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on issue #30910:
URL: https://github.com/apache/airflow/issues/30910#issuecomment-1646035896
I think it's being implemented in https://github.com/apache/airflow/pull/32102
--
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