[GitHub] [airflow] zhongjiajie commented on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] zhongjiajie commented on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] zhongjiajie commented on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
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 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] zhongjiajie commented on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] zhongjiajie commented on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] zhongjiajie commented on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] zhongjiajie commented on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] zhongjiajie commented on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] zhongjiajie commented on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] zhongjiajie commented on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] zhongjiajie commented on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] zhongjiajie commented on issue #7683: [AIRFLOW-7033] Change dag and task state meanwhile
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: us...@infra.apache.org With regards, Apache Git Services
