Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AIRFLOW-3569] Add "Trigger DAG" button in DAG page #4373

Merged
merged 1 commit into from
Jan 14, 2019

Conversation

XD-DENG
Copy link
Member

@XD-DENG XD-DENG commented Dec 26, 2018

Jira

Description

  • Here are some details about my PR, including screenshots of any UI changes:

What this PR is addressing?

Currently, the (manual) Trigger DAG button is only available at the /home page.

But I always encounter this situation: when I'm checking the tree view, or code, or graph view of a specific DAG, I may want to manually trigger another DagRun. I need to go back to the home page, search for that DAG, trigger the DagRun, then click it again to check the tree view.

It may be good if we can have the Manual Trigger DAG button in the DAG page as well, rather than only in the home page.

Others

  1. This PR only updated /www_rbac. Given deprecating old UI (/www) is already in the plan ([AIRFLOW-3303] Deprecate old UI in favor of FAB #4339), updating /www may add extra re-basing work for [AIRFLOW-3303] Deprecate old UI in favor of FAB #4339. But kindly let me know if any committer finds it's still necessary to update /www.

  2. After the DagRun is triggered, users will be re-directed back to the Tree-View tab of the DAG that was triggered.

Screenshots

screen shot 2018-12-26 at 8 07 16 pm

screen shot 2018-12-26 at 7 52 30 pm

To have the Manual Trigger DAG button in the DAG page as well,
rather than only in the home page.
@XD-DENG XD-DENG force-pushed the add_trigger_button_in_dag_page branch from f350974 to 53806ee Compare December 26, 2018 12:10
@codecov-io
Copy link

codecov-io commented Dec 26, 2018

Codecov Report

Merging #4373 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4373   +/-   ##
=======================================
  Coverage   78.14%   78.14%           
=======================================
  Files         202      202           
  Lines       16486    16486           
=======================================
  Hits        12883    12883           
  Misses       3603     3603

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c014324...53806ee. Read the comment docs.

@XD-DENG
Copy link
Member Author

XD-DENG commented Dec 31, 2018

@ashb @kaxil @feng-tao PTAL and advise your thoughts?

@feng-tao
Copy link
Member

thanks for the change. But I am not sure if I like this change. 1. From UI perspective, there will be two places to do the triggering dag which is kinda redundant; 2. The DAG page is a page for DAG information, not for operation(delete dag run, modify dag run, trigger dag run are in a different page). If we add this button, I think people may also argue whether we should add a stop dag run, modify dag run etc button(which are operations related to the dag) to this page.

Just my 2cents.

@XD-DENG
Copy link
Member Author

XD-DENG commented Dec 31, 2018

Thanks @feng-tao . Fully understand your point/concern.

However, I think from UI perspective it may still be worth adding it, in order to be consistent with the Link column in the homepage (screenshot below).
screen shot 2018-12-31 at 7 47 40 pm

But I also agree the value added by this change is not that big.

I would like to leave the decision to you & other committers. Thanks! And happy new year!

@feluelle
Copy link
Member

feluelle commented Jan 9, 2019

@feng-tao got good points on why not to do this but I have to agree to @XD-DENG we could at least have the same Links in the homepage.

Moreover the Delete button is also in the DAG Page AND in the homepage AND it is an interaction. So either I would think about removing the Delete button from the DAG Page or adding the Trigger Dag button to the DAG Page to be consistent.

@feng-tao
Copy link
Member

feng-tao commented Jan 9, 2019

@XD-DENG , @feluelle , fair point :) In this case, I am neutral for this pr. cc @ashb who ships the delete dag UI pr and see his takes. But I think we need to have a consensus on what we should put inside vs outside of the DAG info UI page.

@XD-DENG
Copy link
Member Author

XD-DENG commented Jan 12, 2019

Hi @ashb , a gentle ping: may you advise on this UI change PR? Thanks.

@ashb
Copy link
Member

ashb commented Jan 14, 2019

I think it makes sense to have the same links on the list and detail pages - and haven't been annoyed by the lack of trigger, but I have wanted other options that were missing, so I'm +1

@ashb ashb merged commit 17ff1ed into apache:master Jan 14, 2019
shubhamod pushed a commit to shubhamod/airflow that referenced this pull request Jan 14, 2019
…pache#4373)

To have the Manual Trigger DAG button in the DAG page as well,
rather than only in the home page.
@XD-DENG XD-DENG deleted the add_trigger_button_in_dag_page branch January 27, 2019 14:20
jxiao0 pushed a commit to jxiao0/airflow that referenced this pull request Feb 14, 2019
…pache#4373)

To have the Manual Trigger DAG button in the DAG page as well,
rather than only in the home page.
ashb pushed a commit to ashb/airflow that referenced this pull request Mar 5, 2019
…pache#4373)

To have the Manual Trigger DAG button in the DAG page as well,
rather than only in the home page.
ashb added a commit to ashb/airflow that referenced this pull request Mar 5, 2019
…he#4373)

This is a back-ported version of the change introducted to the RBAC ui
in apache#4373.
wmorris75 pushed a commit to modmed/incubator-airflow that referenced this pull request Jul 29, 2019
…pache#4373)

To have the Manual Trigger DAG button in the DAG page as well,
rather than only in the home page.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants