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/02/14 07:51:42 UTC
[GitHub] [airflow] Jorricks opened a new pull request #21555: Add queue button to click-on-DagRun interface.
Jorricks opened a new pull request #21555:
URL: https://github.com/apache/airflow/pull/21555
<!--
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/
-->
In our use-case for Airflow, we often add tasks after the DAG has been created and had a first couple successful runs.
This means, we often want successful DagRuns to be re-queued, such that it adheres to the limit of number of active DagRuns and that it will create all the TaskInstances for the new tasks.
This option was currently not easily accessible and this makes it more accessible.
---
**^ 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] tanelk edited a comment on pull request #21555: Add queue button to click-on-DagRun interface.
Posted by GitBox <gi...@apache.org>.
tanelk edited a comment on pull request #21555:
URL: https://github.com/apache/airflow/pull/21555#issuecomment-1060621766
You are changing the state to 'queued', but I think the correnct state would be 'scheduled'. The `SchedulerJob` should be only place, where tasks are queued.
scheduled - all task dependencies are met
queued - all resource dependecies are met and the task is handed over to the executor.
In UI the word "queue" might sound more intuitive.
--
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] Jorricks commented on pull request #21555: Add queue button to click-on-DagRun interface.
Posted by GitBox <gi...@apache.org>.
Jorricks commented on pull request #21555:
URL: https://github.com/apache/airflow/pull/21555#issuecomment-1054480124
> I think we need to explain the difference between Clear and Queue. They are both a re-run. Clear just reruns the existing task instances while Queue would add new task instances if the DAG's tasks have changed. Is that right?
That is a perfect description sir. However, the challenge is getting that in a couple words in a button xD
--
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] Jorricks commented on pull request #21555: Add queue button to click-on-DagRun interface.
Posted by GitBox <gi...@apache.org>.
Jorricks commented on pull request #21555:
URL: https://github.com/apache/airflow/pull/21555#issuecomment-1046597846
Well.. I feel "Mark Running" doesn't really fit. Marking the DagRun as Running directly could potentially overwrite the active DagRun limit (which I intentionally want to steer away from).
I don't fully agree on the statement that it might be misleading and people expect you to set all tasks to Queued. When I would put "Mark Running", you would also not expect all Tasks to be started right? So how would that be different for Queued?
However, I do agree that just "Queue" might not do the job. "Mark Queued" might be better but also not really sure...
--
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] bbovenzi commented on pull request #21555: Add queue button to click-on-DagRun interface.
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on pull request #21555:
URL: https://github.com/apache/airflow/pull/21555#issuecomment-1054502381
Maybe we have a new section called "Rerun" with two buttons "Soft rerun" and "Hard rerun". We can also include an info tooltip either next to "Rerun" or next to each button. Thoughts?
--
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 #21555: Add queue button to click-on-DagRun interface.
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #21555:
URL: https://github.com/apache/airflow/pull/21555#issuecomment-1064332107
The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main 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] Jorricks commented on pull request #21555: Add queue button to click-on-DagRun interface.
Posted by GitBox <gi...@apache.org>.
Jorricks commented on pull request #21555:
URL: https://github.com/apache/airflow/pull/21555#issuecomment-1046724797
There is a Queued DagRun state since Airflow 2.2.0 I believe.
--
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] Jorricks commented on pull request #21555: Add queue button to click-on-DagRun interface.
Posted by GitBox <gi...@apache.org>.
Jorricks commented on pull request #21555:
URL: https://github.com/apache/airflow/pull/21555#issuecomment-1056957206
> Maybe we have a new section called "Rerun" with two buttons "Soft rerun" and "Hard rerun". We can also include an info tooltip either next to "Rerun" or next to each button. Thoughts?
I am a bit unsure about that.. I am not a UX designer so wouldn't know what the best way to do this is.
--
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] tanelk commented on pull request #21555: Add queue button to click-on-DagRun interface.
Posted by GitBox <gi...@apache.org>.
tanelk commented on pull request #21555:
URL: https://github.com/apache/airflow/pull/21555#issuecomment-1060621766
You are changing the state to 'queued', but I think the correnct state would be 'scheduled'. The `SchedulerJob` should be only place, where tasks are queued.
--
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] Jorricks commented on a change in pull request #21555: Add queue button to click-on-DagRun interface.
Posted by GitBox <gi...@apache.org>.
Jorricks commented on a change in pull request #21555:
URL: https://github.com/apache/airflow/pull/21555#discussion_r816093359
##########
File path: airflow/www/views.py
##########
@@ -2101,6 +2133,22 @@ def dagrun_success(self):
origin = get_safe_url(request.form.get('origin'))
return self._mark_dagrun_state_as_success(dag_id, dag_run_id, confirmed, origin)
+ @expose('/dagrun_queued', methods=['POST'])
+ @auth.has_access(
+ [
+ (permissions.ACTION_CAN_EDIT, permissions.RESOURCE_DAG),
+ (permissions.ACTION_CAN_EDIT, permissions.RESOURCE_DAG_RUN),
+ ]
+ )
+ @action_logging
+ def dagrun_queued(self):
+ """Mark DagRun success"""
Review comment:
```suggestion
"""Queue DagRun so tasks that haven't run yet can be started."""
```
--
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] bbovenzi commented on pull request #21555: Add queue button to click-on-DagRun interface.
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on pull request #21555:
URL: https://github.com/apache/airflow/pull/21555#issuecomment-1058506920
> I am a bit unsure about that.. I am not a UX designer so wouldn't know what the best way to do this is.
No worries. I'll pull down your branch and give it a try.
--
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] bbovenzi merged pull request #21555: Add queue button to click-on-DagRun interface.
Posted by GitBox <gi...@apache.org>.
bbovenzi merged pull request #21555:
URL: https://github.com/apache/airflow/pull/21555
--
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] eladkal commented on pull request #21555: Add queue button to click-on-DagRun interface.
Posted by GitBox <gi...@apache.org>.
eladkal commented on pull request #21555:
URL: https://github.com/apache/airflow/pull/21555#issuecomment-1046201897
> Note: I choose to add only Queue and not Running as running could potentially go over the limit of allowed running DagRuns.
I would prefer to call the action by it's name. Queue is not so self explanatory and also kinda misleading as you are not really setting the tasks to Queue state. What about "Mark Running" which in essence this is what the action does. It make the DAGRun as running and the scheduler will pick up the tasks.
--
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] Jorricks commented on pull request #21555: Add queue button to click-on-DagRun interface.
Posted by GitBox <gi...@apache.org>.
Jorricks commented on pull request #21555:
URL: https://github.com/apache/airflow/pull/21555#issuecomment-1060624272
> You are changing the state to 'queued', but I think the correnct state would be 'scheduled'. The `SchedulerJob` should be only place, where tasks are queued.
>
> scheduled - all task dependencies are met
> queued - all resource dependecies are met and the task is handed over to the executor.
Please read the comments above.. We are talking about changing the DagRun state, not the task..🙄
--
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] eladkal commented on pull request #21555: Add queue button to click-on-DagRun interface.
Posted by GitBox <gi...@apache.org>.
eladkal commented on pull request #21555:
URL: https://github.com/apache/airflow/pull/21555#issuecomment-1046616149
> However, I do agree that just "Queue" might not do the job. "Mark Queued" might be better but also not really sure...
The action you are taking is on DAGRun object. This is what the user expect. To my perspective since there is no such thing as Queue state for DagRuns it can cause confusion. Specifically because Task Queue state means that it already passed the scheduler and was eligible for running, waiting for worker slot - which is not what you want here.
Lets see what other thinks :)
--
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] bbovenzi commented on pull request #21555: Add queue button to click-on-DagRun interface.
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on pull request #21555:
URL: https://github.com/apache/airflow/pull/21555#issuecomment-1054355129
I think we need to explain the difference between Clear and Queue. They are both a re-run. Clear just reruns the existing task instances while Queue would add new task instances if the DAG's tasks have changed. Is that right?
--
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] Jorricks commented on pull request #21555: Add queue button to click-on-DagRun interface.
Posted by GitBox <gi...@apache.org>.
Jorricks commented on pull request #21555:
URL: https://github.com/apache/airflow/pull/21555#issuecomment-1046199617
Would still love some feedback. Have a good weekend all :v:
--
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] Jorricks edited a comment on pull request #21555: Add queue button to click-on-DagRun interface.
Posted by GitBox <gi...@apache.org>.
Jorricks edited a comment on pull request #21555:
URL: https://github.com/apache/airflow/pull/21555#issuecomment-1046724797
There is a Queued DagRun state since Airflow 2.2.0 I believe.
Link: https://github.com/apache/airflow/blob/2.2.4/airflow/models/dagrun.py#L86
--
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] tanelk edited a comment on pull request #21555: Add queue button to click-on-DagRun interface.
Posted by GitBox <gi...@apache.org>.
tanelk edited a comment on pull request #21555:
URL: https://github.com/apache/airflow/pull/21555#issuecomment-1060621766
You are changing the state to 'queued', but I think the correnct state would be 'scheduled'. The `SchedulerJob` should be only place, where tasks are queued.
scheduled - all task dependencies are met
queued - all resource dependecies are met and the task is handed over to the executor.
--
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] eladkal edited a comment on pull request #21555: Add queue button to click-on-DagRun interface.
Posted by GitBox <gi...@apache.org>.
eladkal edited a comment on pull request #21555:
URL: https://github.com/apache/airflow/pull/21555#issuecomment-1046616149
> However, I do agree that just "Queue" might not do the job. "Mark Queued" might be better but also not really sure...
The action you are taking is on DAGRun object. This is what the user expect. To my perspective since there is no such thing as Queue state for DagRuns it can cause confusion. Specifically because Task Queue state means that it already passed the scheduler and was eligible for running, waiting for worker slot - which is not what you want here.
Lets see what others think :)
--
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] bbovenzi commented on a change in pull request #21555: Add queue button to click-on-DagRun interface.
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on a change in pull request #21555:
URL: https://github.com/apache/airflow/pull/21555#discussion_r815972781
##########
File path: airflow/www/views.py
##########
@@ -2101,6 +2133,22 @@ def dagrun_success(self):
origin = get_safe_url(request.form.get('origin'))
return self._mark_dagrun_state_as_success(dag_id, dag_run_id, confirmed, origin)
+ @expose('/dagrun_queued', methods=['POST'])
+ @auth.has_access(
+ [
+ (permissions.ACTION_CAN_EDIT, permissions.RESOURCE_DAG),
+ (permissions.ACTION_CAN_EDIT, permissions.RESOURCE_DAG_RUN),
+ ]
+ )
+ @action_logging
+ def dagrun_queued(self):
+ """Mark DagRun success"""
Review comment:
Let's update this description
--
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] bbovenzi commented on pull request #21555: Add queue button to click-on-DagRun interface.
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on pull request #21555:
URL: https://github.com/apache/airflow/pull/21555#issuecomment-1059375507
I updated the Run Modal UI:
<img width="606" alt="Screen Shot 2022-03-04 at 12 35 14 PM" src="https://user-images.githubusercontent.com/4600967/156813390-4a005dea-d79c-40e8-ab33-8b4723a9cefa.png">
--
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