[GitHub] [airflow] zhongjiajie edited a comment on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile

2020-04-14 Thread GitBox
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:
us...@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

2020-04-06 Thread GitBox
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:
us...@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

2020-04-06 Thread GitBox
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:
us...@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

2020-03-11 Thread GitBox
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:
us...@infra.apache.org


With regards,
Apache Git Services