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 2021/09/09 02:27:34 UTC

[GitHub] [airflow] rachelwigell opened a new pull request #18107: Make auto refresh interval configurable

rachelwigell opened a new pull request #18107:
URL: https://github.com/apache/airflow/pull/18107

closes: #18069

We accidentally DDOS'd our own webserver yesterday :smile: More details in the issue on how exactly this happened, but the gist is that auto-refresh can be a significant strain if there are many active tasks whose statuses must be polled. Auto-refresh is a wonderful feature but we wanted to be able to lengthen the interval to protect against this.

On main, the interval is hard-coded to 3 seconds. I'm proposing we add a new webserver config variable that will allow this interval to be customized.

<!--
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 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 change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+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 [UPDATING.md](https://github.com/apache/airflow/blob/main/UPDATING.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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] ashb commented on a change in pull request #18107: Make auto refresh interval configurable

Posted by GitBox <gi...@apache.org>.

ashb commented on a change in pull request #18107:
URL: https://github.com/apache/airflow/pull/18107#discussion_r705440520



##########
File path: airflow/www/templates/airflow/graph.html
##########
@@ -128,6 +128,7 @@
     const edges = {{ edges|tojson }};
     const tasks = {{ tasks|tojson }};
     let taskInstances = {{ task_instances|tojson }};
+    const autoRefreshInterval = {{ auto_refresh_interval }};

Review comment:
       It's not risky in this case as the value comes from a trusted source, not user input.




-- 
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] mik-laj commented on a change in pull request #18107: Make auto refresh interval configurable

Posted by GitBox <gi...@apache.org>.

mik-laj commented on a change in pull request #18107:
URL: https://github.com/apache/airflow/pull/18107#discussion_r705007315



##########
File path: airflow/www/templates/airflow/graph.html
##########
@@ -128,6 +128,7 @@
     const edges = {{ edges|tojson }};
     const tasks = {{ tasks|tojson }};
     let taskInstances = {{ task_instances|tojson }};
+    const autoRefreshInterval = {{ auto_refresh_interval }};

Review comment:
       Dynamically generated javascript is risky. Can you use `getMetaValue` method instead?




-- 
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] github-actions[bot] commented on pull request #18107: Make auto refresh interval configurable

Posted by GitBox <gi...@apache.org>.

github-actions[bot] commented on pull request #18107:
URL: https://github.com/apache/airflow/pull/18107#issuecomment-916226172


   The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.


-- 
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] boring-cyborg[bot] commented on pull request #18107: Make auto refresh interval configurable

Posted by GitBox <gi...@apache.org>.

boring-cyborg[bot] commented on pull request #18107:
URL: https://github.com/apache/airflow/pull/18107#issuecomment-915709872

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
Here are some useful points:
- Pay attention to the quality of your code (flake8, mypy and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/main/STATIC_CODE_CHECKS.rst#prerequisites-for-pre-commit-hooks) will help you with that.
- In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/main/docs/apache-airflow/howto/custom-operator.rst) Consider adding an example DAG that shows how users should use it.
- Consider using [Breeze environment](https://github.com/apache/airflow/blob/main/BREEZE.rst) for testing locally, it’s a heavy docker but it ships with a working Airflow and a lot of integrations.
- Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
- Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
- Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#coding-style-and-best-practices).
Apache Airflow is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://s.apache.org/airflow-slack

--
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] rachelwigell commented on pull request #18107: Make auto refresh interval configurable

Posted by GitBox <gi...@apache.org>.

rachelwigell commented on pull request #18107:
URL: https://github.com/apache/airflow/pull/18107#issuecomment-916337648


   > Along with this, I think it would be good to have an option (like a drop-down) near the auto-refresh control to set the refresh rate by the users themselves, which will be stored into browser's local storage or a cookie and can control a user's choice to refresh the data.
   
   I agree that would be a convenient feature, but undermines the original intent of making this change which was to allow admins of an Airflow server to set a longer interval for auto-refresh so that users wouldn't be as easily able to overwhelm the webserver with large numbers of requests.


-- 
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] boring-cyborg[bot] commented on pull request #18107: Make auto refresh interval configurable

Posted by GitBox <gi...@apache.org>.

boring-cyborg[bot] commented on pull request #18107:
URL: https://github.com/apache/airflow/pull/18107#issuecomment-917252712


   Awesome work, congrats on your first merged pull request!
   


-- 
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] msumit commented on pull request #18107: Make auto refresh interval configurable

Posted by GitBox <gi...@apache.org>.

msumit commented on pull request #18107:
URL: https://github.com/apache/airflow/pull/18107#issuecomment-915772966


   This is nice 👍. Along with this, I think it would be good to have an option (like a drop-down) near the auto-refresh control to set the refresh rate by the users themselves, which will be stored into browser's local storage or a cookie and can control a user's choice to refresh the data. 


-- 
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] kaxil merged pull request #18107: Make auto refresh interval configurable

Posted by GitBox <gi...@apache.org>.

kaxil merged pull request #18107:
URL: https://github.com/apache/airflow/pull/18107


   


-- 
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