Skip to content

Support for sorting DAGs in the web UI#22671

Merged
bbovenzi merged 6 commits into
apache:mainfrom
pierrejeambrun:8459-sort-dag-list-home-page
Apr 9, 2022
Merged

Support for sorting DAGs in the web UI#22671
bbovenzi merged 6 commits into
apache:mainfrom
pierrejeambrun:8459-sort-dag-list-home-page

Conversation

@pierrejeambrun

@pierrejeambrun pierrejeambrun commented Mar 31, 2022

Copy link
Copy Markdown
Member

Fixes: #8459

Based on the closed PR #11652, add supports for sorting dag list in the home page.

Try to simplified the logic and added a small test to check that items appear in the correct order in the dom. (dag table)
screen1
screen4
screen2
screen3

@boring-cyborg boring-cyborg Bot added area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues labels Mar 31, 2022
@pierrejeambrun pierrejeambrun changed the title Add sort + small test Support for sorting DAGs in the web UI Mar 31, 2022
bbovenzi
bbovenzi reviewed Mar 31, 2022
View reviewed changes
Comment thread airflow/www/templates/airflow/dags.html Outdated
jbampton
jbampton reviewed Apr 1, 2022
View reviewed changes
Comment thread airflow/www/views.py Outdated
Comment thread airflow/www/views.py Outdated
uranusjr
uranusjr reviewed Apr 1, 2022
View reviewed changes
Comment thread airflow/www/views.py Outdated
uranusjr
uranusjr reviewed Apr 1, 2022
View reviewed changes
Comment thread airflow/www/views.py Outdated
@pierrejeambrun

pierrejeambrun commented Apr 3, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

Thank you for the reviews 👍

I have updated the PR, let me know what you guys think.

Best,

@pierrejeambrun
pierrejeambrun force-pushed the 8459-sort-dag-list-home-page branch 2 times, most recently from be483ce to a4edd71 Compare April 3, 2022 15:28
@pierrejeambrun
pierrejeambrun force-pushed the 8459-sort-dag-list-home-page branch from a4edd71 to 67a85cf Compare April 4, 2022 20:50
@pierrejeambrun

pierrejeambrun commented Apr 4, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

nullslast func is creating an sql syntax error with mysql. An alternative as order_by(column.is_(None), column) is creating a syntax error with mssql.

I am removing it from now. (nulls will appear first in descending order).

@pierrejeambrun
pierrejeambrun force-pushed the 8459-sort-dag-list-home-page branch from 67a85cf to 94f492a Compare April 6, 2022 06:04
@eladkal eladkal added this to the Airflow 2.3.0 milestone Apr 6, 2022
bbovenzi
bbovenzi approved these changes Apr 6, 2022
View reviewed changes

@bbovenzi bbovenzi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good!

The page reloading after every sort isn't a great UX. But I think that's mainly a limitation of using Flask App Builder.

@github-actions github-actions Bot added the okay to merge It's ok to merge this PR as it does not require more tests label Apr 6, 2022
@github-actions

github-actions Bot commented Apr 6, 2022

Copy link
Copy Markdown
Contributor

The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.

uranusjr
uranusjr reviewed Apr 7, 2022
View reviewed changes
Comment thread airflow/www/views.py Outdated
uranusjr
uranusjr approved these changes Apr 9, 2022
View reviewed changes

@uranusjr uranusjr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to go as far as I can tell.

@pierrejeambrun

pierrejeambrun commented Apr 9, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

Looking good!

The page reloading after every sort isn't a great UX. But I think that's mainly a limitation of using Flask App Builder.

Looking forward to the new React app 😄

@bbovenzi
bbovenzi merged commit c0157e6 into apache:main Apr 9, 2022
@pierrejeambrun
pierrejeambrun deleted the 8459-sort-dag-list-home-page branch April 9, 2022 16:35
@ephraimbuddy ephraimbuddy added the type:improvement Changelog: Improvements label Apr 11, 2022
@TryHarder01 TryHarder01 mentioned this pull request Apr 19, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@uranusjr uranusjr uranusjr approved these changes
@bbovenzi bbovenzi bbovenzi approved these changes
@ryanahamilton ryanahamilton Awaiting requested review from ryanahamilton
@ashb ashb Awaiting requested review from ashb
+1 more reviewer
@jbampton jbampton jbampton left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues okay to merge It's ok to merge this PR as it does not require more tests type:improvement Changelog: Improvements

Projects

None yet

Milestone

Airflow 2.3.0

Development

Successfully merging this pull request may close these issues.

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

6 participants

@pierrejeambrun @jbampton @uranusjr @bbovenzi @ephraimbuddy @eladkal