Support for sorting DAGs in the web UI by scrdest · Pull Request #11652 · apache/airflow

scrdest · 2020-10-19T08:38:41Z

Adds support for sorting DAGs in the UI by the ID, Schedule (next run, really) and Owner, ASC/DESC.

The sorting is done on the DB level and parameterized by two new URL parameters - sortBy for the model attribute & orderBy for the type of ordering (ascending/descending).

The default behavior is to sort by Dag ID and order by ascending, and therefore should not come as a surprise to current users.

The column headings in the DAG view have been turned to hyperlinks that pass the parameters for the sorting. For more predictable UX, when switching before sorting attributes (or when none is passed), clicking on a header will always sort it by ASC first. Clicking on the same header again after the page reloads will toggle it to DESC instead. Visually:

(None)->[Dag ID] => (Dag ID ASC)
(Dag ID ASC) -> [Dag ID] => (Dag ID DESC)
(Dag ID DESC) -> [Dag ID] => (Dag ID ASC)
(Dag ID ASC) -> [Owner] => (Owner ASC)
(Dag ID DESC) -> [Owner] => (Owner ASC)

Unfortunately, sorting by the most recently executed run as requested by #8459 is not supported yet. The current options were fairly straightforward to handle since the logic that populates the DAG list currently fetches a set of DagModels - this is just an exercise in filtering. Fetching and sorting DagRuns ran the risk of adding a fair bit of extra processing overhead and complicating the logic.

closes: #8459

Screenshots:

boring-cyborg · 2020-10-19T08:38:45Z

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst)
Here are some useful points:

Pay attention to the quality of your code (flake8, pylint and type annotations). Our pre-commits will help you with that.
In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
Consider using Breeze environment for testing locally, it’s a heavy docker but it ships with a working Airflow and a lot of integrations.
Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
Be sure to read the Airflow Coding style.
Apache Airflow is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: [email protected]
Slack: https://s.apache.org/airflow-slack

ashb · 2020-10-19T13:55:12Z

/cc @ryanahamilton

ryanahamilton

This will be a nice update! I believe there used to be a JavaScript implementation of this, but it required the full dataset to be loaded (not paginated) in order to sort properly. Speaking of pagination, I think you need to add sortBy and orderBy arguments to wwwutils.generate_pages() in views.py so those params aren't abandoned on subsequent pages.

ryanahamilton · 2020-10-21T18:40:17Z

Nice, I like the changing icons based on the sort state!

ashb · 2020-10-23T20:55:25Z

It might be easier/less code to have a single sort_by and have this as

Suggested change

dag_query_key_map = {

'dag_id': DagModel.dag_id,

'owner': DagModel.owners,

'schedule': DagModel.next_dagrun,

# <- add any extra (URL param)->(DagModel attr) mappings here

}

dag_query_key_map = {

'dag_id': DagModel.dag_id,

'-dag_id': DagModel.dag_id.desc(),

'owner': DagModel.owners,

'-owner': DagModel.owners.desc(),

'schedule': DagModel.next_dagrun,

'-schedule': DagModel.next_dagrun.desc(),

# <- add any extra (URL param)->(DagModel attr) mappings here

}

That way we don't need to have extra sorting_order param. I think this makes the code and the URI simpler. (For instance, query_ordering_transform_for_key wouldn't be needed anymore)

Additionally this order probably doesn't belong in this file -- but instead in the views.py file, probably in the home function.

(If you think the current way is clearer, tell me. But I do think the location is wrong for this, as it is specific to one particular view.)

I see your point, but I feel like the suggested way is a bigger PITA from a long-term maintenance perspective - you have to add each new sortable in duplicate. It also seems less intuitive when looking at the URL - the current setup basically just replicates the underlying SQL. All in all, I was trying to make the two bits as independently testable as possible.

I may be getting ahead of myself, but it should also make things easier if someone wanted to sort by multiple columns - e.g. ASC by ID and DESC by owner would be orderBy=asc;desc and then you could just zip() it with sortBy's items.

Re: the module location, point taken. I will move things around as appropriate. I was kind of hoping someone would slap me and point me to the right place if this one wasn't it, so - appreciated!

ryw · 2020-10-27T19:40:56Z

thanks for working on this @scrdest and your first Airflow contribution :)

ryanahamilton · 2020-10-30T13:54:09Z

@scrdest heads up, I've made several recent markup changes to the table in dag.html that are yielding conflicts. Should be pretty straightforward to resolve!

kaxil · 2020-11-04T00:26:38Z

Can you please rebase your PR on latest Master since we have applied Black and PyUpgrade on Master.

It will help if your squash your commits into single commit first so that there are less conflicts.

scrdest · 2020-11-05T08:48:15Z

@kaxil - rebased.

ryanahamilton

I pulled this down and tested it locally—it's looking really good. Left a couple suggestions.

github-actions · 2020-11-17T09:19:17Z

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

github-actions · 2020-11-17T15:54:13Z

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

github-actions · 2020-11-17T15:55:29Z

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

github-actions · 2020-11-23T15:08:54Z

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

potiuk · 2020-12-07T10:13:09Z

Long thread :) - I think however that one is not critical and can be done in 2.1 ? @ashb @ryanahamilton WDYT?

…status to the web UI.

…) to views and moved the tests correspondingly.

Co-authored-by: RosterIn <[email protected]>

Co-authored-by: Ryan Hamilton <[email protected]>

…he roles of the two params.

…un" to keep it in line with the underlying attribute.

jkmalek · 2021-08-04T14:08:10Z

@jedcunningham rebased and fixed up where necessary; thanks for the heads-up on view tests!

jedcunningham

The pagination links also aren't maintaining orderBy and orderDir, but I haven't dug into why. Do they work for you?

jedcunningham · 2021-08-18T20:39:53Z

+    dag_id_query = dag_query_for_key(sorting_key='dag_id')
+    dag_id_query_casetest = dag_query_for_key(sorting_key='dAg_Id')
+
+    assert dag_id_query == DagModel.dag_id
+    assert dag_id_query_casetest == dag_id_query
+
+    owner_query = dag_query_for_key(sorting_key='owner')
+    owner_query_casetest = dag_query_for_key(sorting_key='oWnEr')
+
+    assert owner_query == DagModel.owners
+    assert owner_query != dag_id_query
+    assert owner_query_casetest == owner_query
+
+    schedule_query = dag_query_for_key(sorting_key='next_dagrun')
+
+    assert schedule_query == DagModel.next_dagrun
+    assert schedule_query != dag_id_query
+    assert schedule_query != owner_query


nit: This whole section could utilize pytest.mark.parameterize which would make it cleaner imo, e.g. (untested):

Suggested change

dag_id_query = dag_query_for_key(sorting_key='dag_id')

dag_id_query_casetest = dag_query_for_key(sorting_key='dAg_Id')

assert dag_id_query == DagModel.dag_id

assert dag_id_query_casetest == dag_id_query

owner_query = dag_query_for_key(sorting_key='owner')

owner_query_casetest = dag_query_for_key(sorting_key='oWnEr')

assert owner_query == DagModel.owners

assert owner_query != dag_id_query

assert owner_query_casetest == owner_query

schedule_query = dag_query_for_key(sorting_key='next_dagrun')

assert schedule_query == DagModel.next_dagrun

assert schedule_query != dag_id_query

assert schedule_query != owner_query

@pytest.mark.parameterize(

"key, expected_query",

[

["dag_id", DagModel.dag_id],

["dAg_Id", DagModel.dag_id],

["owner", DagModel.owners],

["oWnEr", DagModel.owners],

["next_dagrun", DagModel.next_dagrun],

["NEXT_DAGRUN", DagModel.next_dagrun],

]

)

def test_dag_query_for_key(key, expected_query):

assert dag_query_for_key(sorting_key=key) == expected_query

The same pattern could be used for query_ordering_transform_for_key and build_dag_sorting_query.

jedcunningham · 2021-08-18T21:24:47Z

+   {% set curr_ordering = ('unsorted' if request.args.get('orderDir') != attr_name else request_ordering ) %}
+   {% set new_ordering = ('asc' if request.args.get('orderDir') != attr_name or curr_ordering != 'asc' else 'desc' ) %}


Suggested change

{% set curr_ordering = ('unsorted' if request.args.get('orderDir') != attr_name else request_ordering ) %}

{% set new_ordering = ('asc' if request.args.get('orderDir') != attr_name or curr_ordering != 'asc' else 'desc' ) %}

{% set curr_ordering = ('unsorted' if request.args.get('orderBy') != attr_name else request_ordering ) %}

{% set new_ordering = ('asc' if request.args.get('orderBy') != attr_name or curr_ordering != 'asc' else 'desc' ) %}

kaxil · 2021-09-14T11:49:15Z

@scrdest Can you address the requested changes please

github-actions · 2021-10-30T00:06:14Z

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.

boring-cyborg Bot added the area:webserver Webserver related Issues label Oct 19, 2020

ryanahamilton requested changes Oct 19, 2020

View reviewed changes

Comment thread airflow/www/templates/airflow/dags.html Outdated

Comment thread airflow/www/templates/airflow/dags.html Outdated

Comment thread airflow/www/templates/airflow/dags.html Outdated

Comment thread airflow/www/templates/airflow/dags.html Outdated

jkmalek force-pushed the 8459-sorting-dags branch from 8069a9a to bc75866 Compare October 22, 2020 14:10

ashb requested changes Oct 23, 2020

View reviewed changes

Comment thread airflow/www/templates/airflow/dags.html Outdated

ashb reviewed Oct 23, 2020

View reviewed changes

jkmalek force-pushed the 8459-sorting-dags branch from bc75866 to dc67ed0 Compare October 28, 2020 16:08

jkmalek force-pushed the 8459-sorting-dags branch from dc67ed0 to 8d62206 Compare October 31, 2020 15:36

jkmalek force-pushed the 8459-sorting-dags branch from 8d62206 to 217d26b Compare November 5, 2020 08:44

ryanahamilton requested changes Nov 5, 2020

View reviewed changes

Comment thread airflow/www/templates/airflow/dags.html Outdated

Comment thread airflow/www/templates/airflow/dags.html Outdated

jkmalek force-pushed the 8459-sorting-dags branch from aa0be27 to e79ddbf Compare November 17, 2020 08:31

ryanahamilton added the area:UI Related to UI/UX. For Frontend Developers. label Nov 18, 2020

kaxil requested a review from ryanahamilton November 19, 2020 12:50

jkmalek force-pushed the 8459-sorting-dags branch from d459503 to 11a7232 Compare November 23, 2020 14:38

kaxil added this to the Airflow 2.0.0 (rc1) milestone Nov 26, 2020

ryanahamilton reviewed Dec 3, 2020

View reviewed changes

Comment thread airflow/www/templates/airflow/dags.html Outdated

ryanahamilton approved these changes Dec 3, 2020

View reviewed changes

ryanahamilton requested a review from ashb December 3, 2020 14:02

ashb requested changes Dec 3, 2020

View reviewed changes

Comment thread airflow/www/views.py Outdated

Comment thread airflow/www/views.py Outdated

jkmalek and others added 12 commits August 4, 2021 08:26

Simplified the sorting logic (at the cost of an explicit import)

1b0204d

Propagated the sorting to the paginator. Added icons for the sorting …

241967e

…status to the web UI.

triggering rebuild due to timeout

671cb7d

Macro-ified the sorting in dags.html. Moved the sorting helpers (back…

5feb86b

…) to views and moved the tests correspondingly.

Rebased on master.

e76a129

Re-blackened the changes.

170e67e

Another rebase + suggested changes to tooltips.

5f5a78a

Update tests/www/test_views.py

5d4552b

Co-authored-by: RosterIn <[email protected]>

Update airflow/www/templates/airflow/dags.html

59777ea

Co-authored-by: Ryan Hamilton <[email protected]>

Renames sortBy to orderBy and former orderBy to orderDir to clarify t…

ff30be9

…he roles of the two params.

Fix syntax to pass Black static check

7c756f0

Renamed the sorting key for next_dagrun from "schedule" to "next_dagr…

68efcce

…un" to keep it in line with the underlying attribute.

jkmalek force-pushed the 8459-sorting-dags branch from 2de1a14 to e522fd7 Compare August 4, 2021 11:02

Another rebase + refactored added test case to pytest.

5585e10

jkmalek force-pushed the 8459-sorting-dags branch from e522fd7 to 5585e10 Compare August 4, 2021 11:19

jkmalek added 2 commits August 5, 2021 08:51

Fixed up the linter complaints.

fdbd2f3

...ACTUALLY fixes up the static checks (missed some).

6877460

jedcunningham requested changes Aug 18, 2021

View reviewed changes

bbovenzi mentioned this pull request Aug 25, 2021

Add another tab in Dags table that shows running now #17826

Closed

kaxil modified the milestones: Airflow 2.2, Airflow 2.3 Sep 14, 2021

bbovenzi self-requested a review September 14, 2021 14:26

github-actions Bot added the stale Stale PRs per the .github/workflows/stale.yml policy file label Oct 30, 2021

github-actions Bot closed this Nov 4, 2021

uranusjr mentioned this pull request Feb 18, 2022

Add ability to sort dag list in the UI by Dag, Owner, Last Run #8459

Closed

pierrejeambrun mentioned this pull request Mar 31, 2022

Support for sorting DAGs in the web UI #22671

Merged

tquazi mentioned this pull request Apr 28, 2023

Support for sorting DAGs by Last Run Date in the web UI #30935

Merged

-    dag_query_key_map = {
-        'dag_id': DagModel.dag_id,
-        'owner': DagModel.owners,
-        'schedule': DagModel.next_dagrun,
-        # <- add any extra (URL param)->(DagModel attr) mappings here
-    }
+    dag_query_key_map = {
+        'dag_id': DagModel.dag_id,
+        '-dag_id': DagModel.dag_id.desc(),
+        'owner': DagModel.owners,
+        '-owner': DagModel.owners.desc(),
+        'schedule': DagModel.next_dagrun,
+        '-schedule': DagModel.next_dagrun.desc(),
+        # <- add any extra (URL param)->(DagModel attr) mappings here
+    }

		{% set curr_ordering = ('unsorted' if request.args.get('orderDir') != attr_name else request_ordering ) %}
		{% set new_ordering = ('asc' if request.args.get('orderDir') != attr_name or curr_ordering != 'asc' else 'desc' ) %}

Uh oh!

Conversation

scrdest commented Oct 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

boring-cyborg Bot commented Oct 19, 2020

Uh oh!

ashb commented Oct 19, 2020

Uh oh!

ryanahamilton left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ryanahamilton commented Oct 21, 2020

Uh oh!

Uh oh!

ashb Oct 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ashb Oct 23, 2020

Choose a reason for hiding this comment

Uh oh!

scrdest Oct 26, 2020

Choose a reason for hiding this comment

Uh oh!

ryw commented Oct 27, 2020

Uh oh!

ryanahamilton commented Oct 30, 2020

Uh oh!

kaxil commented Nov 4, 2020

Uh oh!

scrdest commented Nov 5, 2020

Uh oh!

ryanahamilton left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Nov 17, 2020

Uh oh!

github-actions Bot commented Nov 17, 2020

Uh oh!

github-actions Bot commented Nov 17, 2020

Uh oh!

github-actions Bot commented Nov 23, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

potiuk commented Dec 7, 2020

Uh oh!

jkmalek commented Aug 4, 2021

Uh oh!

jedcunningham left a comment

Choose a reason for hiding this comment

Uh oh!

jedcunningham Aug 18, 2021

Choose a reason for hiding this comment

Uh oh!

jedcunningham Aug 18, 2021

Choose a reason for hiding this comment

Uh oh!

kaxil commented Sep 14, 2021

Uh oh!

github-actions Bot commented Oct 30, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

scrdest commented Oct 19, 2020 •

edited

Loading

ashb Oct 23, 2020 •

edited

Loading