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 2020/12/13 07:21:29 UTC
[GitHub] [airflow] yuqian90 opened a new pull request #13037: Add a "Resume" option to Mark Success to automatically resume blocked tasks after marking success
yuqian90 opened a new pull request #13037:
URL: https://github.com/apache/airflow/pull/13037
closes: #12485
---
When marking a failed task as success in Graph View, this PR gives the user an additional checkbox "Resume". When checked, this option automatically clears downstream tasks that were blocked by the failed task after marking it success.
For example, given a DagRun currently in this state, with ``run_after_loop`` and ``run_this_last`` blocked by the failed task ``runme_1``:
When the user marks ``runme_1`` as success with "Resume" checked, downstream tasks in ``upstream_failed`` state are automatically cleared:
The DagRun looks like this after clicking OK:
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #13037: Add a "Resume" option to Mark Success to automatically resume blocked tasks after marking success
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#discussion_r542372722
##########
File path: airflow/www/templates/airflow/dag.html
##########
@@ -310,6 +310,10 @@ <h4>Task Actions</h4>
<input type="checkbox" value="true" name="success_downstream" autocomplete="off">
Downstream
</label>
+ <label class="btn btn-default btn-sm active" title="Automatically clear blocked downstream tasks">
Review comment:
```suggestion
<label class="btn btn-default btn-sm active" title="Automatically clear downstream tasks that were in 'upstream_failed' state">
```
(If that's actually what it does?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #13037: Marking success/failed automatically clears failed downstream tasks
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#discussion_r643068739
##########
File path: airflow/models/dag.py
##########
@@ -1153,6 +1153,7 @@ def clear(
max_recursion_depth=None,
dag_bag=None,
visited_external_tis=None,
+ exclude_task_ids: FrozenSet[str] = frozenset({}),
Review comment:
This also shouldn't be a frozen set (or even _any_ set) -- but a `typing.Container[str]`
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on pull request #13037: Add a "Resume" option to Mark Success to automatically resume blocked tasks after marking success
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#issuecomment-744001794
Could you please add a note in documentation (tbh not sure where)?
On side note, would it make sens to have a tooltip showing small description for all options in ti view? CC @ryanahamilton
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #13037: Marking success/failed automatically clears failed downstream tasks
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#discussion_r637971893
##########
File path: UPDATING.md
##########
@@ -167,6 +167,10 @@ constantly.
From Airflow 2.0.0, the scheduling decisions have been moved from
DagFileProcessor to Scheduler, so we can keep the default a bit higher: `30`.
+### Marking success/failed automatically clears failed downstream tasks
Review comment:
This section now needs moving to the top
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] yuqian90 commented on pull request #13037: Marking success/failed automatically clears failed downstream tasks
Posted by GitBox <gi...@apache.org>.
yuqian90 commented on pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#issuecomment-864382271
> I don't mind if we don't change the behaviour of the deprecated API (and probably we shouldn't change it) but so long as the OpeAPI and UI behave the same I'm good.
Hi @ash, please see this PR that addresses your comment: https://github.com/apache/airflow/pull/16539
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #13037: Marking success/failed automatically clears failed downstream tasks
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#issuecomment-758561485
Can you please rebase @yuqian90 ? We have some serious CI slow-downs (we are working on it) but this leads to some master broken more often than we would like to have. Apologies. Rebasing should fix the problem.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] yuqian90 commented on pull request #13037: Marking success/failed automatically clears failed downstream tasks
Posted by GitBox <gi...@apache.org>.
yuqian90 commented on pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#issuecomment-854619884
> @yuqian90 @kaxil This need tackling in a different way -- there shouldn't be _any_ logic in the view, as that means that the Web UI and the API behave differently.
Hi @ashb . I can't seem to find an API function that has similar features as `_mark_task_instance_state()` in `views.py`. If we don't put this logic in `views.py`, should i move it into `airflow.api.common.experimental.mark_tasks.set_state()` ?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #13037: Marking success/failed automatically clears failed downstream tasks
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#issuecomment-758568719
> Thanks. Rebased and picked up this commit from upstream/master:
>
> ```
> commit 8ecdef3e50d3b83901d70a13794ae6afabc4964e (upstream/master)
> Author: Kaxil Naik <ka...@gmail.com>
> Date: Tue Jan 12 10:16:01 2021 +0000
>
> Audit Log records View should not contain link if dag_id is None (#13619)
That too. It's the parent of this one that fixes the problem (:crossed_fingers: )
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #13037: Marking success/failed automatically clears failed downstream tasks
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#issuecomment-758286718
[The Workflow run](https://github.com/apache/airflow/actions/runs/477679076) is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ryanahamilton commented on pull request #13037: Add a "Resume" option to Mark Success to automatically resume blocked tasks after marking success
Posted by GitBox <gi...@apache.org>.
ryanahamilton commented on pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#issuecomment-744469197
I agree with @ashb that adding another button wouldn't be ideal. If we _do_ need to add one, the current placement isn't quite appropriate within that button group since the other options are for task selection. Something like this would make more sense IMO:
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #13037: Marking success/failed automatically clears failed downstream tasks
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#issuecomment-758549681
[The Workflow run](https://github.com/apache/airflow/actions/runs/479755386) is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil merged pull request #13037: Marking success/failed automatically clears failed downstream tasks
Posted by GitBox <gi...@apache.org>.
kaxil merged pull request #13037:
URL: https://github.com/apache/airflow/pull/13037
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #13037: Add a "Resume" option to Mark Success to automatically resume blocked tasks after marking success
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#discussion_r542373983
##########
File path: airflow/www/views.py
##########
@@ -1741,16 +1742,44 @@ def _mark_task_instance_state( # pylint: disable=too-many-arguments
from airflow.api.common.experimental.mark_tasks import set_state
if confirmed:
- altered = set_state(
- tasks=[task],
- execution_date=execution_date,
- upstream=upstream,
- downstream=downstream,
- future=future,
- past=past,
- state=state,
- commit=True,
- )
+ with create_session() as session:
+ altered = set_state(
+ tasks=[task],
+ execution_date=execution_date,
+ upstream=upstream,
+ downstream=downstream,
+ future=future,
+ past=past,
+ state=state,
+ commit=True,
+ session=session,
+ )
+
+ if state == State.SUCCESS and resume:
+ # If Resume is checked when marking success, clear downstream tasks that
+ # are in upstream_failed state to resume them.
+
+ # Flush the session so that the tasks marked success are reflected in the db.
+ session.flush()
+ subdag = dag.sub_dag(
Review comment:
This is the deprecated name for it
```suggestion
partial_dag = dag.partial_subset(
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on pull request #13037: Add a "Resume" option to Mark Success to automatically resume blocked tasks after marking success
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#issuecomment-744430587
Is there a case when you _wouldn't_ want this behaviour? If we can not add a new checkbox to that already monsterously complex modal I (and Ryan!) would be happier.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] yuqian90 commented on pull request #13037: Add a "Resume" option to Mark Success to automatically resume blocked tasks after marking success
Posted by GitBox <gi...@apache.org>.
yuqian90 commented on pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#issuecomment-744132511
> Could you please add a note in documentation (tbh not sure where)?
>
> On side note, would it make sens to have a tooltip showing small description for all options in ti view? CC @ryanahamilton
Hi @turbaszek . Thanks. I've added a section in [ui.rst](https://github.com/apache/airflow/blob/dab11f2d5e0229e01b3bb75582bd3b5ba8b7f1a9/docs/apache-airflow/ui.rst#task-instance-context-menu) under "Task Instance Context Menu". I also added a note in UPDATING.md about this feature.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] yuqian90 commented on pull request #13037: Marking success/failed automatically clears failed downstream tasks
Posted by GitBox <gi...@apache.org>.
yuqian90 commented on pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#issuecomment-854695302
> @yuqian90
>
> https://github.com/apache/airflow/blob/75c91b4acf1ed45d6ccf60a6e1326700233a4f05/airflow/api_connexion/endpoints/task_instance_endpoint.py#L272
>
> is the comparable endpoint in the API isn't it?
Thanks. That's a good point. Should we change `airflow.api.common.experimental.mark_tasks.set_state` so that we can make both `_mark_task_instance_state()` and `post_set_task_instances_state` clear downstream tasks in `upstream_failed` state when a failed task is marked success?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] yuqian90 commented on a change in pull request #13037: Add a "Resume" option to Mark Success to automatically resume blocked tasks after marking success
Posted by GitBox <gi...@apache.org>.
yuqian90 commented on a change in pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#discussion_r555057714
##########
File path: airflow/www/templates/airflow/dag.html
##########
@@ -310,6 +310,10 @@ <h4>Task Actions</h4>
<input type="checkbox" value="true" name="success_downstream" autocomplete="off">
Downstream
</label>
+ <label class="btn btn-default btn-sm active" title="Automatically clear blocked downstream tasks">
Review comment:
This change is no longer needed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] RosterIn commented on pull request #13037: Add a "Resume" option to Mark Success to automatically resume blocked tasks after marking success
Posted by GitBox <gi...@apache.org>.
RosterIn commented on pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#issuecomment-744187248
@yuqian90 I think this button can also apply to Mark Failed.
If the downstream task has trigger rule of one_failed / all_failed etc
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] yuqian90 commented on a change in pull request #13037: Add a "Resume" option to Mark Success to automatically resume blocked tasks after marking success
Posted by GitBox <gi...@apache.org>.
yuqian90 commented on a change in pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#discussion_r555058016
##########
File path: airflow/www/views.py
##########
@@ -1741,16 +1742,44 @@ def _mark_task_instance_state( # pylint: disable=too-many-arguments
from airflow.api.common.experimental.mark_tasks import set_state
if confirmed:
- altered = set_state(
- tasks=[task],
- execution_date=execution_date,
- upstream=upstream,
- downstream=downstream,
- future=future,
- past=past,
- state=state,
- commit=True,
- )
+ with create_session() as session:
+ altered = set_state(
+ tasks=[task],
+ execution_date=execution_date,
+ upstream=upstream,
+ downstream=downstream,
+ future=future,
+ past=past,
+ state=state,
+ commit=True,
+ session=session,
+ )
+
+ if state == State.SUCCESS and resume:
+ # If Resume is checked when marking success, clear downstream tasks that
+ # are in upstream_failed state to resume them.
+
+ # Flush the session so that the tasks marked success are reflected in the db.
+ session.flush()
+ subdag = dag.sub_dag(
+ task_ids_or_regex=fr"^{task_id}$",
+ include_downstream=True,
+ include_upstream=False,
+ )
+
+ end_date = execution_date if not future else None
+ start_date = execution_date if not past else None
+
+ subdag.clear(
+ start_date=start_date,
+ end_date=end_date,
+ include_subdags=True,
+ include_parentdag=True,
+ only_failed=True,
Review comment:
Thanks. Updated doc.
##########
File path: airflow/www/views.py
##########
@@ -1741,16 +1742,44 @@ def _mark_task_instance_state( # pylint: disable=too-many-arguments
from airflow.api.common.experimental.mark_tasks import set_state
if confirmed:
- altered = set_state(
- tasks=[task],
- execution_date=execution_date,
- upstream=upstream,
- downstream=downstream,
- future=future,
- past=past,
- state=state,
- commit=True,
- )
+ with create_session() as session:
+ altered = set_state(
+ tasks=[task],
+ execution_date=execution_date,
+ upstream=upstream,
+ downstream=downstream,
+ future=future,
+ past=past,
+ state=state,
+ commit=True,
+ session=session,
+ )
+
+ if state == State.SUCCESS and resume:
+ # If Resume is checked when marking success, clear downstream tasks that
+ # are in upstream_failed state to resume them.
+
+ # Flush the session so that the tasks marked success are reflected in the db.
+ session.flush()
+ subdag = dag.sub_dag(
+ task_ids_or_regex=fr"^{task_id}$",
Review comment:
Done.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #13037: Marking success/failed automatically clears failed downstream tasks
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#issuecomment-799013613
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] yuqian90 commented on pull request #13037: Add a "Resume" option to Mark Success to automatically resume blocked tasks after marking success
Posted by GitBox <gi...@apache.org>.
yuqian90 commented on pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#issuecomment-744485230
Thanks @ashb and @ryanahamilton
I see two slightly different suggestions:
1. Do away with an extra button and make "Success + Resume" the default and only behaviour.
2. Add the extra "Success+Resume" button like @ryanahamilton suggested, instead of adding a checkbox.
I'm fine with either. I'm tempted to go with 1 because I'm trying hard to think of a scenario where we do not want this behaviour. Does anyone have a usecase in mind where we want to mark a task success but not clear downstream failed?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on pull request #13037: Marking success/failed automatically clears failed downstream tasks
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#issuecomment-854630974
@yuqian90 https://github.com/apache/airflow/blob/75c91b4acf1ed45d6ccf60a6e1326700233a4f05/airflow/api_connexion/endpoints/task_instance_endpoint.py#L272 is the comparable endpoint in the API isn't it?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #13037: Marking success/failed automatically clears failed downstream tasks
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#discussion_r637973164
##########
File path: airflow/www/views.py
##########
@@ -1811,16 +1811,43 @@ def _mark_task_instance_state( # pylint: disable=too-many-arguments
from airflow.api.common.experimental.mark_tasks import set_state
if confirmed:
- altered = set_state(
- tasks=[task],
- execution_date=execution_date,
- upstream=upstream,
- downstream=downstream,
- future=future,
- past=past,
- state=state,
- commit=True,
- )
+ with create_session() as session:
Review comment:
Can you add tests for it @yuqian90
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #13037: Marking success/failed automatically clears failed downstream tasks
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#issuecomment-758651259
[The Workflow run](https://github.com/apache/airflow/actions/runs/479969578) is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] yuqian90 commented on a change in pull request #13037: Add a "Resume" option to Mark Success to automatically resume blocked tasks after marking success
Posted by GitBox <gi...@apache.org>.
yuqian90 commented on a change in pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#discussion_r555057859
##########
File path: airflow/www/views.py
##########
@@ -1741,16 +1742,44 @@ def _mark_task_instance_state( # pylint: disable=too-many-arguments
from airflow.api.common.experimental.mark_tasks import set_state
if confirmed:
- altered = set_state(
- tasks=[task],
- execution_date=execution_date,
- upstream=upstream,
- downstream=downstream,
- future=future,
- past=past,
- state=state,
- commit=True,
- )
+ with create_session() as session:
+ altered = set_state(
+ tasks=[task],
+ execution_date=execution_date,
+ upstream=upstream,
+ downstream=downstream,
+ future=future,
+ past=past,
+ state=state,
+ commit=True,
+ session=session,
+ )
+
+ if state == State.SUCCESS and resume:
+ # If Resume is checked when marking success, clear downstream tasks that
+ # are in upstream_failed state to resume them.
+
+ # Flush the session so that the tasks marked success are reflected in the db.
+ session.flush()
+ subdag = dag.sub_dag(
Review comment:
Thanks. Done
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #13037: Marking success/failed automatically clears failed downstream tasks
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#discussion_r566165755
##########
File path: UPDATING.md
##########
@@ -52,6 +52,10 @@ assists users migrating to a new version.
## Master
+### Marking success/failed automatically clears failed downstream tasks
+
+When marking a task success/failed in Graph View, its downstream tasks that are in failed/upstream_failed state are automatically cleared.
+
Review comment:
I think 2.1
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on a change in pull request #13037: Marking success/failed automatically clears failed downstream tasks
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#discussion_r555565506
##########
File path: UPDATING.md
##########
@@ -52,6 +52,10 @@ assists users migrating to a new version.
## Master
+### Marking success/failed automatically clears failed downstream tasks
+
+When marking a task success/failed in Graph View, its downstream tasks that are in failed/upstream_failed state are automatically cleared.
+
Review comment:
@kaxil @ashb is this a 2.1 or 3.0 change? The behaviour is changing but it should not broke DAGs or pipelines (I 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] yuqian90 commented on pull request #13037: Add a "Resume" option to Mark Success to automatically resume blocked tasks after marking success
Posted by GitBox <gi...@apache.org>.
yuqian90 commented on pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#issuecomment-744382431
> @yuqian90 I think this button can also apply to Mark Failed.
> If the downstream task has trigger rule of one_failed / all_failed etc
Hi @RosterIn I agree it's a possible use case. However, supporting that actually requires some more work. The current PR is using this convenience method `DAG.clear()` like this. After marking the task success, the `only_failed=True` argument makes sure that task does not get cleared by this line of code here. However, to use the same code for Mark Failed means that it's going to clear the task itself (that was just marked failed).
```python
subdag.clear(
start_date=start_date,
end_date=end_date,
include_subdags=True,
include_parentdag=True,
only_failed=True,
session=session,
)
```
To Mark Failed work work with Resume is not difficult, probably just need to add another argument to `DAG.clear()` to exclude the task itself or something like that. That requires more work and testing.
That said, `one_failed` or `all_failed` are much less common than `all_success`. So I'm tempted to doing that in another PR.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on pull request #13037: Add a "Resume" option to Mark Success to automatically resume blocked tasks after marking success
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#issuecomment-745195104
I certainly can't think of one, 1 sounds good to me.
Question: does the (new) API use the same code path? Perhaps this could be a case where the API could have the parameter, but the UI is kept simpler.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #13037: Add a "Resume" option to Mark Success to automatically resume blocked tasks after marking success
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#discussion_r542373452
##########
File path: airflow/www/views.py
##########
@@ -1741,16 +1742,44 @@ def _mark_task_instance_state( # pylint: disable=too-many-arguments
from airflow.api.common.experimental.mark_tasks import set_state
if confirmed:
- altered = set_state(
- tasks=[task],
- execution_date=execution_date,
- upstream=upstream,
- downstream=downstream,
- future=future,
- past=past,
- state=state,
- commit=True,
- )
+ with create_session() as session:
+ altered = set_state(
+ tasks=[task],
+ execution_date=execution_date,
+ upstream=upstream,
+ downstream=downstream,
+ future=future,
+ past=past,
+ state=state,
+ commit=True,
+ session=session,
+ )
+
+ if state == State.SUCCESS and resume:
+ # If Resume is checked when marking success, clear downstream tasks that
+ # are in upstream_failed state to resume them.
+
+ # Flush the session so that the tasks marked success are reflected in the db.
+ session.flush()
+ subdag = dag.sub_dag(
+ task_ids_or_regex=fr"^{task_id}$",
Review comment:
```suggestion
task_ids_or_regex=[task_id],
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] yuqian90 commented on pull request #13037: Marking success/failed automatically clears failed downstream tasks
Posted by GitBox <gi...@apache.org>.
yuqian90 commented on pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#issuecomment-758563998
> Can you please rebase @yuqian90 ? We have some serious CI slow-downs (we are working on it) but this leads to some master broken more often than we would like to have. Apologies. Rebasing should fix the problem.
Thanks. Rebased and picked up this commit from upstream/master:
```
commit 8ecdef3e50d3b83901d70a13794ae6afabc4964e (upstream/master)
Author: Kaxil Naik <ka...@gmail.com>
Date: Tue Jan 12 10:16:01 2021 +0000
Audit Log records View should not contain link if dag_id is None (#13619)
closes https://github.com/apache/airflow/issues/13602
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on pull request #13037: Marking success/failed automatically clears failed downstream tasks
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#issuecomment-854844891
I don't mind if we don't change the behaviour of the deprecated API (and probably we shouldn't change it) but so long as the OpeAPI and UI behave the same I'm good.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] yuqian90 commented on pull request #13037: Add a "Resume" option to Mark Success to automatically resume blocked tasks after marking success
Posted by GitBox <gi...@apache.org>.
yuqian90 commented on pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#issuecomment-757968126
> @yuqian90 I think this button can also apply to Mark Failed.
> If the downstream task has trigger rule of one_failed / all_failed etc
@RosterIn This is now done. Marking Success/Failed will both clear downstream tasks in failed/upstream_failed state.
> I certainly can't think of one, 1 sounds good to me.
>
> Question: does the (new) API use the same code path? Perhaps this could be a case where the API could have the parameter, but the UI is kept simpler.
HI @ash I went with option 1. The PR now looks much simpler. Please take another look. I have not looked into the API. It's not using the code path in views.py as far as I can tell. I think API changes can be done separately.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #13037: Add a "Resume" option to Mark Success to automatically resume blocked tasks after marking success
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#discussion_r542374344
##########
File path: airflow/www/views.py
##########
@@ -1741,16 +1742,44 @@ def _mark_task_instance_state( # pylint: disable=too-many-arguments
from airflow.api.common.experimental.mark_tasks import set_state
if confirmed:
- altered = set_state(
- tasks=[task],
- execution_date=execution_date,
- upstream=upstream,
- downstream=downstream,
- future=future,
- past=past,
- state=state,
- commit=True,
- )
+ with create_session() as session:
+ altered = set_state(
+ tasks=[task],
+ execution_date=execution_date,
+ upstream=upstream,
+ downstream=downstream,
+ future=future,
+ past=past,
+ state=state,
+ commit=True,
+ session=session,
+ )
+
+ if state == State.SUCCESS and resume:
+ # If Resume is checked when marking success, clear downstream tasks that
+ # are in upstream_failed state to resume them.
+
+ # Flush the session so that the tasks marked success are reflected in the db.
+ session.flush()
+ subdag = dag.sub_dag(
+ task_ids_or_regex=fr"^{task_id}$",
+ include_downstream=True,
+ include_upstream=False,
+ )
+
+ end_date = execution_date if not future else None
+ start_date = execution_date if not past else None
+
+ subdag.clear(
+ start_date=start_date,
+ end_date=end_date,
+ include_subdags=True,
+ include_parentdag=True,
+ only_failed=True,
Review comment:
This is technically going to clear FAILED as well as UPSTREAM_FAILED.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on pull request #13037: Marking success/failed automatically clears failed downstream tasks
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#issuecomment-758568719
> Thanks. Rebased and picked up this commit from upstream/master:
>
> ```
> commit 8ecdef3e50d3b83901d70a13794ae6afabc4964e (upstream/master)
> Author: Kaxil Naik <ka...@gmail.com>
> Date: Tue Jan 12 10:16:01 2021 +0000
>
> Audit Log records View should not contain link if dag_id is None (#13619)
That too. It's the parent of this one that fixes the problem (:crossed_fingers: )
```
commit eb40eea81be95ecd0e71807145797b6d82375885
Author: Jarek Potiuk <ja...@polidea.com>
Date: Tue Jan 12 10:25:52 2021 +0100
Add submodules in added workflow job (#13631)
The most recent submodule change for actions #13514 was done in
parallel to Optimising worklfows in #13562 and the job added in
the #13562 still uses non-submodule version of check action.
Also few checkout steps missed:
'submodules: recursive' input
This PR fixes that and all 3rd-party actions now are used
from submodule.
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #13037: Marking success/failed automatically clears failed downstream tasks
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #13037:
URL: https://github.com/apache/airflow/pull/13037#discussion_r637972215
##########
File path: airflow/models/dag.py
##########
@@ -1190,6 +1191,8 @@ def clear(
:param visited_external_tis: A set used internally to keep track of the visited TaskInstance when
clearing tasks across multiple DAGs linked by ExternalTaskMarker to avoid redundant work.
:type visited_external_tis: set
+ :param exclude_task_ids: A set of task_id that should not be cleared
Review comment:
```suggestion
:param exclude_task_ids: A set of ``task_id``'s that should not be cleared
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org