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/03/10 16:37:04 UTC
[GitHub] [airflow] zhongjiajie opened a new pull request #7683:
[AIRFLOW-7033] Change dag and task state meanwhile
zhongjiajie opened a new pull request #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683
In dag page, have make task instance state to success
or failed bottom, when we click those bottom not only
change task instance state but also dag run state
if possible.
* When trigger make_failed bottom, the dag run state
should change to failed
* When trigger make_success bottom, then dag run
state should change to success if all task instance
state are success, otherwise should keep the
original state
---
Issue link: WILL BE INSERTED BY [boring-cyborg](https://github.com/kaxil/boring-cyborg)
Make sure to mark the boxes below before creating PR: [x]
- [x] Description above provides context of the change
- [x] Commit message/PR title starts with `[AIRFLOW-NNNN]`. AIRFLOW-NNNN = JIRA ID<sup>*</sup>
- [x] Unit tests coverage for changes (not needed for documentation changes)
- [x] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
- [x] Relevant documentation is updated including usage instructions.
- [x] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
<sup>*</sup> For document-only changes commit message can start with `[AIRFLOW-XXXX]`.
---
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/master/UPDATING.md).
Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on issue #7683: [AIRFLOW-7033]
Change dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#issuecomment-613830825
Ok, I will try to
* Remove "DAG" button in Task Instance dialog in Graph view
* Add task instances in dag run dialog in Tree view
But I insist on add function `_set_dag_run_state_according_ti` in `mark_tasks.py` and use `include_dag=True` as default value in `view.py`, that mean mark dagrun success if all task instance success as default behavior
```diff
return self._mark_task_instance_state(dag_id, task_id, origin, execution_date,
confirmed, upstream, downstream,
future, past, State.FAILED)
- future, past, State.FAILED)
+ future, past, include_dag=True, State.FAILED)
```
Cause I think It's a missing function in Task Instance Modal dialog, I have some situation about this dialog. For example
```py
task1 >> [task2, task3]
```
* **CASE ONE**: task2 and task3 failed, and task3 **not need** to re-run(not related about time and next scheduler task work fine), but task2 **have to be rerun**. I just mark task3 success and clear task2 include downstream. every thing work fine and dagrun mark as success after finish task2.
* **CASE TWO**: task2 and task3 failed and both of them **not need to rerun**, I just click task1 and mark task instance success include downstream, but dagrun still with failed status. Yeah we could go to tree view mark dagrun success after I add optional button beside 'mark success/failed' button, but it's mean I have to move from graph to tree view, which meaningless step.
So I think change dagrun state in Task Instance Modal dialog if possible make sense, and I want to keep it this function. And remove the "DAG" button in Task Instance Modal dialog to avoid misleading to user. WDYT @feluelle @kaxil @potiuk
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on issue #7683: [AIRFLOW-7033]
Change dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#issuecomment-599186252
Fix conflict
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] feluelle commented on issue #7683: [AIRFLOW-7033] Change
dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
feluelle commented on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#issuecomment-613503822
No, I mean on the tree view page. So you can click on one dag run and can mark this dagrun and maybe future or past ones (as succeeded or 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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #7683:
[AIRFLOW-7033] Change dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#issuecomment-597665730
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=h1) Report
> Merging [#7683](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/271ee6477b80633049457557020a4f525f9135cd&el=desc) will **decrease** coverage by `0.00%`.
> The diff coverage is `91.30%`.
[](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7683 +/- ##
==========================================
- Coverage 86.93% 86.92% -0.01%
==========================================
Files 915 915
Lines 44158 44168 +10
==========================================
+ Hits 38387 38395 +8
- Misses 5771 5773 +2
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/api/common/experimental/mark\_tasks.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy9hcGkvY29tbW9uL2V4cGVyaW1lbnRhbC9tYXJrX3Rhc2tzLnB5) | `94.51% <90.47%> (-1.01%)` | :arrow_down: |
| [airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=) | `76.30% <100.00%> (+0.03%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=footer). Last update [271ee64...035c685](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #7683:
[AIRFLOW-7033] Change dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#issuecomment-597665730
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=h1) Report
> Merging [#7683](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/271ee6477b80633049457557020a4f525f9135cd&el=desc) will **decrease** coverage by `0.00%`.
> The diff coverage is `91.30%`.
[](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7683 +/- ##
==========================================
- Coverage 86.93% 86.92% -0.01%
==========================================
Files 915 915
Lines 44158 44168 +10
==========================================
+ Hits 38387 38395 +8
- Misses 5771 5773 +2
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/api/common/experimental/mark\_tasks.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy9hcGkvY29tbW9uL2V4cGVyaW1lbnRhbC9tYXJrX3Rhc2tzLnB5) | `94.51% <90.47%> (-1.01%)` | :arrow_down: |
| [airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=) | `76.30% <100.00%> (+0.03%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=footer). Last update [271ee64...035c685](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #7683:
[AIRFLOW-7033] Change dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#issuecomment-597665730
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=h1) Report
> Merging [#7683](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/271ee6477b80633049457557020a4f525f9135cd&el=desc) will **decrease** coverage by `0.00%`.
> The diff coverage is `91.30%`.
[](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7683 +/- ##
==========================================
- Coverage 86.93% 86.92% -0.01%
==========================================
Files 915 915
Lines 44158 44168 +10
==========================================
+ Hits 38387 38395 +8
- Misses 5771 5773 +2
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/api/common/experimental/mark\_tasks.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy9hcGkvY29tbW9uL2V4cGVyaW1lbnRhbC9tYXJrX3Rhc2tzLnB5) | `94.51% <90.47%> (-1.01%)` | :arrow_down: |
| [airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=) | `76.30% <100.00%> (+0.03%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=footer). Last update [271ee64...035c685](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #7683:
[AIRFLOW-7033] Change dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#issuecomment-597665730
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=h1) Report
> Merging [#7683](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/271ee6477b80633049457557020a4f525f9135cd&el=desc) will **decrease** coverage by `0.00%`.
> The diff coverage is `91.30%`.
[](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7683 +/- ##
==========================================
- Coverage 86.93% 86.92% -0.01%
==========================================
Files 915 915
Lines 44158 44168 +10
==========================================
+ Hits 38387 38395 +8
- Misses 5771 5773 +2
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/api/common/experimental/mark\_tasks.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy9hcGkvY29tbW9uL2V4cGVyaW1lbnRhbC9tYXJrX3Rhc2tzLnB5) | `94.51% <90.47%> (-1.01%)` | :arrow_down: |
| [airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=) | `76.30% <100.00%> (+0.03%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=footer). Last update [271ee64...035c685](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on issue #7683: [AIRFLOW-7033]
Change dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#issuecomment-613534495
BTW, I just checkout and find out we already have this function in tree view dagrun
<img width="662" alt="Screen Shot 2020-04-15 at 00 09 15" src="https://user-images.githubusercontent.com/15820530/79247663-6197c600-7ead-11ea-8400-658da5d129cf.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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on issue #7683: [AIRFLOW-7033]
Change dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#issuecomment-597473748
> I'm not really sure could we remove this code, it seem unnecessary
Oh, now I realize the code is not unnecessary
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] kaxil commented on issue #7683: [AIRFLOW-7033] Change dag
and task state meanwhile
Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#issuecomment-613737841
> Yes that’s the window I meant.
>
> Don‘t you think it would make more sense to add an optional button "task instances" next to mark success/fail button? instead of "dag" on the task instance dialog window?
I agree with @feluelle , I think it would make more sense to not have this on Task Instance Modal dialog and instead on a specific DAG.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] feluelle commented on issue #7683: [AIRFLOW-7033] Change
dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
feluelle commented on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#issuecomment-613664451
Yes that’s the window I meant.
Don‘t you think it would make more sense to add an optional button „task instances“ next to mark success/fail button? instead of „dag“ on the tsk instance dialog window?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on issue #7683: [AIRFLOW-7033]
Change dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#issuecomment-613831990
And BTW, I alway check DAG view at first, because in big dag with many tasks, I think it's more straightforward about the relationship in dag view instead of tree view.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on a change in pull request #7683:
[AIRFLOW-7033] Change dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on a change in pull request #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#discussion_r390752240
##########
File path: airflow/api/common/experimental/mark_tasks.py
##########
@@ -312,8 +374,6 @@ def set_dag_run_state_to_success(dag, execution_date, commit=False, session=None
_set_dag_run_state(dag.dag_id, execution_date, State.SUCCESS, session)
# Mark all task instances of the dag run to success.
- for task in dag.tasks:
- task.dag = dag
Review comment:
Could we remove this code?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on issue #7683: [AIRFLOW-7033]
Change dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#issuecomment-598619366
@kaxil @potiuk @mik-laj @nuclearpinguin PTAL. Just ignore the conflict, will fix it after review.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on a change in pull request #7683:
[AIRFLOW-7033] Change dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on a change in pull request #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#discussion_r390752331
##########
File path: airflow/api/common/experimental/mark_tasks.py
##########
@@ -340,19 +400,10 @@ def set_dag_run_state_to_failed(dag, execution_date, commit=False, session=None)
_set_dag_run_state(dag.dag_id, execution_date, State.FAILED, session)
# Mark only RUNNING task instances.
- task_ids = [task.task_id for task in dag.tasks]
- tis = session.query(TaskInstance).filter(
+ tasks = session.query(TaskInstance).filter(
TaskInstance.dag_id == dag.dag_id,
- TaskInstance.execution_date == execution_date,
- TaskInstance.task_id.in_(task_ids)).filter(TaskInstance.state == State.RUNNING)
- task_ids_of_running_tis = [task_instance.task_id for task_instance in tis]
-
- tasks = []
- for task in dag.tasks:
- if task.task_id not in task_ids_of_running_tis:
- continue
- task.dag = dag
- tasks.append(task)
Review comment:
And also this one?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #7683:
[AIRFLOW-7033] Change dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#issuecomment-597665730
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=h1) Report
> Merging [#7683](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/2536b517e6b8d9b54289beb7d0da4fee809e0fb5&el=desc) will **decrease** coverage by `0.50%`.
> The diff coverage is `91.30%`.
[](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7683 +/- ##
==========================================
- Coverage 86.98% 86.48% -0.51%
==========================================
Files 904 904
Lines 43736 43745 +9
==========================================
- Hits 38045 37833 -212
- Misses 5691 5912 +221
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/api/common/experimental/mark\_tasks.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy9hcGkvY29tbW9uL2V4cGVyaW1lbnRhbC9tYXJrX3Rhc2tzLnB5) | `94.44% <90.47%> (-1.04%)` | :arrow_down: |
| [airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=) | `76.16% <100.00%> (+0.03%)` | :arrow_up: |
| [...flow/providers/apache/cassandra/hooks/cassandra.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2Nhc3NhbmRyYS9ob29rcy9jYXNzYW5kcmEucHk=) | `21.51% <0.00%> (-72.16%)` | :arrow_down: |
| [...w/providers/apache/hive/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2hpdmUvb3BlcmF0b3JzL215c3FsX3RvX2hpdmUucHk=) | `35.84% <0.00%> (-64.16%)` | :arrow_down: |
| [airflow/providers/postgres/operators/postgres.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcG9zdGdyZXMvb3BlcmF0b3JzL3Bvc3RncmVzLnB5) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [airflow/providers/redis/operators/redis\_publish.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvb3BlcmF0b3JzL3JlZGlzX3B1Ymxpc2gucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [airflow/providers/mongo/sensors/mongo.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbW9uZ28vc2Vuc29ycy9tb25nby5weQ==) | `53.33% <0.00%> (-46.67%)` | :arrow_down: |
| [airflow/providers/mysql/operators/mysql.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbXlzcWwvb3BlcmF0b3JzL215c3FsLnB5) | `55.00% <0.00%> (-45.00%)` | :arrow_down: |
| [airflow/providers/redis/sensors/redis\_key.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvc2Vuc29ycy9yZWRpc19rZXkucHk=) | `61.53% <0.00%> (-38.47%)` | :arrow_down: |
| [airflow/executors/celery\_executor.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvY2VsZXJ5X2V4ZWN1dG9yLnB5) | `50.67% <0.00%> (-37.84%)` | :arrow_down: |
| ... and [7 more](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=footer). Last update [2536b51...dc5fdb3](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #7683:
[AIRFLOW-7033] Change dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#issuecomment-597665730
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=h1) Report
> Merging [#7683](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/271ee6477b80633049457557020a4f525f9135cd&el=desc) will **decrease** coverage by `0.06%`.
> The diff coverage is `91.30%`.
[](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7683 +/- ##
==========================================
- Coverage 86.93% 86.87% -0.07%
==========================================
Files 915 915
Lines 44158 44168 +10
==========================================
- Hits 38387 38369 -18
- Misses 5771 5799 +28
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/api/common/experimental/mark\_tasks.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy9hcGkvY29tbW9uL2V4cGVyaW1lbnRhbC9tYXJrX3Rhc2tzLnB5) | `94.51% <90.47%> (-1.01%)` | :arrow_down: |
| [airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=) | `76.30% <100.00%> (+0.03%)` | :arrow_up: |
| [airflow/security/kerberos.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy9zZWN1cml0eS9rZXJiZXJvcy5weQ==) | `30.43% <0.00%> (-45.66%)` | :arrow_down: |
| [airflow/jobs/backfill\_job.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL2JhY2tmaWxsX2pvYi5weQ==) | `90.98% <0.00%> (-1.13%)` | :arrow_down: |
| [airflow/jobs/scheduler\_job.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL3NjaGVkdWxlcl9qb2IucHk=) | `90.73% <0.00%> (-0.15%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=footer). Last update [271ee64...035c685](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on issue #7683: [AIRFLOW-7033]
Change dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#issuecomment-613443087
ping @kaxil @mik-laj @feluelle again, I need one more approval/review here
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie edited a comment on issue #7683:
[AIRFLOW-7033] Change dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
zhongjiajie edited a comment on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#issuecomment-597473748
> I'm not really sure could we remove this code, it seem unnecessary
Oh, now I realize the code is not unnecessary after CI not happy https://travis-ci.org/github/apache/airflow/builds/660934668?utm_medium=notification&utm_source=github_status
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on issue #7683: [AIRFLOW-7033] Change
dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#issuecomment-603150459
It looks good to me, I just would like to have second opinion (@mik-laj @kaxil @ashb )
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on issue #7683: [AIRFLOW-7033]
Change dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#issuecomment-613532992
@feluelle You'r right, I that another way to mark dag run state. But this PR is try to fix the exists bug(maybe) in tool view in dag/tree view for task instance. The core idea is "should also change dag run state if all task instance success or at least one fail". So I think this two function do not conflict
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on a change in pull request #7683:
[AIRFLOW-7033] Change dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on a change in pull request #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#discussion_r390930257
##########
File path: airflow/api/common/experimental/mark_tasks.py
##########
@@ -340,19 +400,10 @@ def set_dag_run_state_to_failed(dag, execution_date, commit=False, session=None)
_set_dag_run_state(dag.dag_id, execution_date, State.FAILED, session)
# Mark only RUNNING task instances.
- task_ids = [task.task_id for task in dag.tasks]
- tis = session.query(TaskInstance).filter(
+ tasks = session.query(TaskInstance).filter(
TaskInstance.dag_id == dag.dag_id,
- TaskInstance.execution_date == execution_date,
- TaskInstance.task_id.in_(task_ids)).filter(TaskInstance.state == State.RUNNING)
- task_ids_of_running_tis = [task_instance.task_id for task_instance in tis]
-
- tasks = []
- for task in dag.tasks:
- if task.task_id not in task_ids_of_running_tis:
- continue
- task.dag = dag
- tasks.append(task)
Review comment:
I change the code in the next commit
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on issue #7683: [AIRFLOW-7033]
Change dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#issuecomment-601492156
PTAL @ashb @kaxil @potiuk @mik-laj @turbaszek @feluelle
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on issue #7683: [AIRFLOW-7033]
Change dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#issuecomment-609890826
I almost forget about this 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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #7683:
[AIRFLOW-7033] Change dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#issuecomment-597665730
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=h1) Report
> Merging [#7683](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/271ee6477b80633049457557020a4f525f9135cd&el=desc) will **decrease** coverage by `0.00%`.
> The diff coverage is `91.30%`.
[](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7683 +/- ##
==========================================
- Coverage 86.93% 86.92% -0.01%
==========================================
Files 915 915
Lines 44158 44168 +10
==========================================
+ Hits 38387 38395 +8
- Misses 5771 5773 +2
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/api/common/experimental/mark\_tasks.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy9hcGkvY29tbW9uL2V4cGVyaW1lbnRhbC9tYXJrX3Rhc2tzLnB5) | `94.51% <90.47%> (-1.01%)` | :arrow_down: |
| [airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=) | `76.30% <100.00%> (+0.03%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=footer). Last update [271ee64...035c685](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie edited a comment on issue #7683:
[AIRFLOW-7033] Change dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
zhongjiajie edited a comment on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#issuecomment-613830825
Ok, I will try to
* Remove "DAG" button in Task Instance dialog in Graph view
* Add task instances in dag run dialog in Tree view
But I insist on add function `_set_dag_run_state_according_ti` in `mark_tasks.py` and use `include_dag=True` as default value in `view.py`, that mean mark dagrun success if all task instance success as default behavior
```diff
return self._mark_task_instance_state(dag_id, task_id, origin, execution_date,
confirmed, upstream, downstream,
future, past, State.FAILED)
- future, past, State.FAILED)
+ future, past, include_dag=True, State.FAILED)
```
Cause I think It's a missing function in Task Instance Modal dialog, I have some situation about this dialog. For example
```py
task1 >> [task2, task3]
```
* **CASE ONE**: task2 and task3 failed, and task3 **not need** to re-run(not related about time and next scheduler task work fine), but task2 **have to be rerun**. I just mark task3 success and clear task2 include downstream. every thing work fine and dagrun mark as success after finish task2.
* **CASE TWO**: task2 and task3 failed and both of them **not need to rerun**, I just click task1 and mark task instance success include downstream, but dagrun still with failed status. I have to go to dag run page to correct dag run state. Yeah we could go to tree view mark dagrun success after I add optional button beside 'mark success/failed' button, but it's mean I have to move from graph to tree view, which meaningless step.
So I think change dagrun state in Task Instance Modal dialog if possible make sense, and I want to keep it this function. And remove the "DAG" button in Task Instance Modal dialog to avoid misleading to user. WDYT @feluelle @kaxil @potiuk
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie edited a comment on issue #7683:
[AIRFLOW-7033] Change dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
zhongjiajie edited a comment on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#issuecomment-610174787
> This does not look right to me - to put this button on task-level actions window. Wouldn’t it be better to put it on the DagRun actions window (when you click on the round colored button)?
Hi @feluelle , we already have set state to `failed/running/success` in dagrun page. And this feature not trigger from dagrun, but trigger from Dag page when trying to make task failed/success, so I think it should not put in the DagRun page.
This patch will check if all task in dagrun success or not, if all task success will change dagrun state to success, otherwise will keep in to failed.
But I agree with you that it maybe confuse users, with dag in task windows, maybe we should make `include_dag` default `True` in `_mark_task_instance_state`
```py
return self._mark_task_instance_state(dag_id, task_id, origin, execution_date,
confirmed, upstream, downstream,
future, past, State.FAILED)
future, past, include_dag=True, State.FAILED)
```
and change the `UPDATING.md`. I'm left here wait for other committer advices.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #7683:
[AIRFLOW-7033] Change dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#issuecomment-597665730
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=h1) Report
> Merging [#7683](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/271ee6477b80633049457557020a4f525f9135cd&el=desc) will **decrease** coverage by `0.00%`.
> The diff coverage is `91.30%`.
[](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7683 +/- ##
==========================================
- Coverage 86.93% 86.92% -0.01%
==========================================
Files 915 915
Lines 44158 44168 +10
==========================================
+ Hits 38387 38395 +8
- Misses 5771 5773 +2
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/api/common/experimental/mark\_tasks.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy9hcGkvY29tbW9uL2V4cGVyaW1lbnRhbC9tYXJrX3Rhc2tzLnB5) | `94.51% <90.47%> (-1.01%)` | :arrow_down: |
| [airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=) | `76.30% <100.00%> (+0.03%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=footer). Last update [271ee64...035c685](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] feluelle commented on issue #7683: [AIRFLOW-7033] Change
dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
feluelle commented on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#issuecomment-610155710
This does not look right to me - to put this button on task-level actions window. Wouldn’t it be better to put it on the DagRun actions window (when you click on the round colored button)?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on issue #7683: [AIRFLOW-7033]
Change dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#issuecomment-610174787
> This does not look right to me - to put this button on task-level actions window. Wouldn’t it be better to put it on the DagRun actions window (when you click on the round colored button)?
Hi @feluelle , we already have set state to `failed/running/success` in dagrun page. And this feature not trigger from dagrun, but trigger from Dags page when trying to make task failed/success, so I think it should not put in the DagRun page.
This patch will check if all task in dagrun success or not, if all task success will change dagrun state to success, otherwise will keep in to failed.
But I agree with you that it maybe confuse users, with dag in task windows, maybe we should make `include_dag` default `True` in `_mark_task_instance_state`
```py
return self._mark_task_instance_state(dag_id, task_id, origin, execution_date,
confirmed, upstream, downstream,
future, past, State.FAILED)
future, past, include_dag=True, State.FAILED)
```
and change the `UPDATING.md`. I'm left here wait for other committer advices.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie edited a comment on issue #7683:
[AIRFLOW-7033] Change dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
zhongjiajie edited a comment on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#issuecomment-610174787
> This does not look right to me - to put this button on task-level actions window. Wouldn’t it be better to put it on the DagRun actions window (when you click on the round colored button)?
Hi @feluelle , we already have set state to `failed/running/success` in dagrun page. And this feature not trigger from dagrun, but trigger from Dag/Tree page when trying to make task failed/success, so I think it should not put in the DagRun page.
This patch will check if all task in dagrun success or not, if all task success will change dagrun state to success, otherwise will keep in to failed.
But I agree with you that it maybe confuse users, with dag in task windows, maybe we should make `include_dag` default `True` in `_mark_task_instance_state`
```py
return self._mark_task_instance_state(dag_id, task_id, origin, execution_date,
confirmed, upstream, downstream,
future, past, State.FAILED)
future, past, include_dag=True, State.FAILED)
```
and change the `UPDATING.md`. I'm left here wait for other committer advices.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io commented on issue #7683: [AIRFLOW-7033]
Change dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#issuecomment-597665730
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=h1) Report
> Merging [#7683](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/2536b517e6b8d9b54289beb7d0da4fee809e0fb5?src=pr&el=desc) will **decrease** coverage by `22.43%`.
> The diff coverage is `30.43%`.
[](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7683 +/- ##
===========================================
- Coverage 86.98% 64.54% -22.44%
===========================================
Files 904 903 -1
Lines 43736 43732 -4
===========================================
- Hits 38045 28229 -9816
- Misses 5691 15503 +9812
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=) | `25.6% <0%> (-50.53%)` | :arrow_down: |
| [airflow/api/common/experimental/mark\_tasks.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy9hcGkvY29tbW9uL2V4cGVyaW1lbnRhbC9tYXJrX3Rhc2tzLnB5) | `19.13% <33.33%> (-76.35%)` | :arrow_down: |
| [...low/contrib/operators/wasb\_delete\_blob\_operator.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy93YXNiX2RlbGV0ZV9ibG9iX29wZXJhdG9yLnB5) | `0% <0%> (-100%)` | :arrow_down: |
| [airflow/contrib/hooks/vertica\_hook.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL3ZlcnRpY2FfaG9vay5weQ==) | `0% <0%> (-100%)` | :arrow_down: |
| [airflow/contrib/sensors/\_\_init\_\_.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL3NlbnNvcnMvX19pbml0X18ucHk=) | `0% <0%> (-100%)` | :arrow_down: |
| [airflow/hooks/mssql\_hook.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9tc3NxbF9ob29rLnB5) | `0% <0%> (-100%)` | :arrow_down: |
| [...viders/docker/example\_dags/example\_docker\_swarm.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZG9ja2VyL2V4YW1wbGVfZGFncy9leGFtcGxlX2RvY2tlcl9zd2FybS5weQ==) | `0% <0%> (-100%)` | :arrow_down: |
| [airflow/hooks/webhdfs\_hook.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy93ZWJoZGZzX2hvb2sucHk=) | `0% <0%> (-100%)` | :arrow_down: |
| [airflow/contrib/sensors/emr\_base\_sensor.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL3NlbnNvcnMvZW1yX2Jhc2Vfc2Vuc29yLnB5) | `0% <0%> (-100%)` | :arrow_down: |
| [...irflow/contrib/operators/slack\_webhook\_operator.py](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9zbGFja193ZWJob29rX29wZXJhdG9yLnB5) | `0% <0%> (-100%)` | :arrow_down: |
| ... and [488 more](https://codecov.io/gh/apache/airflow/pull/7683/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=footer). Last update [2536b51...dc5fdb3](https://codecov.io/gh/apache/airflow/pull/7683?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #7683:
[AIRFLOW-7033] Change dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#discussion_r397038865
##########
File path: airflow/api/common/experimental/mark_tasks.py
##########
@@ -312,8 +374,6 @@ def set_dag_run_state_to_success(dag, execution_date, commit=False, session=None
_set_dag_run_state(dag.dag_id, execution_date, State.SUCCESS, session)
# Mark all task instances of the dag run to success.
- for task in dag.tasks:
- task.dag = dag
Review comment:
Not sure. @mik-laj -> maybe you can take a look, you know much better now how changing state works and whether this change will be 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
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on issue #7683: [AIRFLOW-7033]
Change dag and task state meanwhile
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
URL: https://github.com/apache/airflow/pull/7683#issuecomment-597399419
Also and make_success/make_failed to dag.html/tree.html
----------------------------------------------------------------
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
With regards,
Apache Git Services