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/11/22 16:19:16 UTC
[GitHub] [airflow] bbovenzi opened a new pull request, #27839: Refresh next run datasets info in dags view
bbovenzi opened a new pull request, #27839:
URL: https://github.com/apache/airflow/pull/27839
The auto-refresh on the dags home page was not updating the next run info for dataset triggered runs. It would stay at "1 of 2 datasets updated" even if both datasets updated and that run was triggered. When opening the modal, that would be correct.
Now during autorefresh we check the datasets requirements for the next run.
---
**^ 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 #27839: Refresh next run datasets info in dags view
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on code in PR #27839:
URL: https://github.com/apache/airflow/pull/27839#discussion_r1030598520
##########
airflow/www/views.py:
##########
@@ -851,6 +851,36 @@ def datasets(self):
state_color_mapping=state_color_mapping,
)
+ @expose("/next_run_datasets_summary", methods=["POST"])
+ @auth.has_access([(permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG)])
+ @provide_session
+ def next_run_datasets_summary(self, session=None):
+ """Next run info for dataset triggered DAGs."""
+ allowed_dag_ids = get_airflow_app().appbuilder.sm.get_accessible_dag_ids(g.user)
+
+ if not allowed_dag_ids:
+ return flask.json.jsonify({})
+
+ # Filter by post parameters
+ selected_dag_ids = {unquote(dag_id) for dag_id in request.form.getlist("dag_ids") if dag_id}
+
+ if selected_dag_ids:
+ filter_dag_ids = selected_dag_ids.intersection(allowed_dag_ids)
+ else:
+ filter_dag_ids = allowed_dag_ids
+
+ dataset_triggered_dag_ids = (
+ session.query(DagModel.dag_id)
+ .filter(DagModel.dag_id.in_(filter_dag_ids))
+ .filter(DagModel.schedule_interval == "Dataset")
+ )
Review Comment:
```suggestion
dataset_triggered_dag_ids = [
dag.dag_id
for dag in
session.query(DagModel.dag_id)
.filter(DagModel.dag_id.in_(filter_dag_ids))
.filter(DagModel.schedule_interval == "Dataset")
]
```
Might need some formatting, but let's do this instead - that way we pass list[str] to get_dataset_triggered_next_run_info like it expects.
##########
airflow/www/static/js/dags.js:
##########
@@ -271,6 +273,25 @@ function dagStatsHandler(selector, json) {
});
}
+function nextRunDatasetsSummaryHandler(json) {
+ [...document.getElementsByClassName('next-dataset-triggered')].forEach((el) => {
+ const dagId = $(el).attr('data-dag-id');
+ const previousSummary = $(el).attr('data-summary');
+ const nextDatasetsInfo = json[dagId];
+
+ // Only update dags that depend on multiple datasets
+ if (!nextDatasetsInfo.uri) {
+ const newSummary = `${nextDatasetsInfo.ready} of ${nextDatasetsInfo.total} datasets updated`;
+
+ // Only updated the element if the summary has changed
Review Comment:
```suggestion
// Only update the element if the summary has changed
```
--
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 merged pull request #27839: Refresh next run datasets info in dags view
Posted by GitBox <gi...@apache.org>.
jedcunningham merged PR #27839:
URL: https://github.com/apache/airflow/pull/27839
--
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 #27839: Refresh next run datasets info in dags view
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on code in PR #27839:
URL: https://github.com/apache/airflow/pull/27839#discussion_r1029693828
##########
airflow/www/templates/airflow/dags.html:
##########
@@ -319,7 +320,7 @@ <h2>{{ page_title }}</h2>
{% if ds_info.total == 1 -%}
On {{ ds_info.uri[0:40] + '…' if ds_info.uri and ds_info.uri|length > 40 else ds_info.uri|default('', true) }}
{%- else -%}
- {{ ds_info.ready }} of {{ ds_info.total }} datasets updated
+ <span class="js-datasets-ready">{{ ds_info.ready }}</span> of <span class="js-datasets-total">{{ ds_info.total }}</span> datasets updated
Review Comment:
Are these new classes (`js-datasets-ready` and `js-datasets-total`) actually used?
--
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 #27839: Refresh next run datasets info in dags view
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on code in PR #27839:
URL: https://github.com/apache/airflow/pull/27839#discussion_r1029689903
##########
airflow/www/views.py:
##########
@@ -851,6 +851,39 @@ def datasets(self):
state_color_mapping=state_color_mapping,
)
+ @expose("/next_run_datasets_summary", methods=["POST"])
+ @auth.has_access([(permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG)])
+ @provide_session
+ def next_run_datasets_summary(self, session=None):
+ """Next run info for dataset triggered DAGs."""
+ allowed_dag_ids = get_airflow_app().appbuilder.sm.get_accessible_dag_ids(g.user)
+
+ if not allowed_dag_ids:
+ return flask.json.jsonify({})
+
+ # Filter by post parameters
+ selected_dag_ids = {unquote(dag_id) for dag_id in request.form.getlist("dag_ids") if dag_id}
+
+ if selected_dag_ids:
+ filter_dag_ids = selected_dag_ids.intersection(allowed_dag_ids)
+ else:
+ filter_dag_ids = allowed_dag_ids
+
+ dataset_triggered_dag_ids = (
+ session.query(DagModel.dag_id)
+ .filter(DagModel.dag_id.in_(filter_dag_ids))
+ .filter(DagModel.schedule_interval == "Dataset")
+ )
+
+ if dataset_triggered_dag_ids:
Review Comment:
This will always be true, since this is just the query. Probably want an `.all()` above.
--
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