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

DAG list sorting lost when switching page #29756

Merged
merged 6 commits into from
Mar 2, 2023

Conversation

amoghrajesh
Copy link
Contributor

The dag list sorting order is lost on changing the pages. We should persist the key across pagination
closes: #29322


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

Copy link
Member

@pierrejeambrun pierrejeambrun left a comment

Choose a reason for hiding this comment

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

You need to test your feature locally before sending that for a review as 'ready to merge' so you are sure this actually solves the problem. You can take a look at these guides to setup your local environment for development:

If the PR is not ready you can mark is a 'draft' while waiting for feedback.

For this particular case, you can take a look at one specific query param that we are handling, for instance 'search' and look how it's done.
(generate_pages now takes those 2 extra args but do nothing with them, look at get_params).

airflow/www/utils.py Show resolved Hide resolved
airflow/www/views.py Outdated Show resolved Hide resolved
@amoghrajesh amoghrajesh changed the title Adding sorting keys to generate_pages DAG list sorting lost when switching page Feb 26, 2023
@amoghrajesh amoghrajesh requested review from pierrejeambrun and removed request for ryanahamilton, ashb and bbovenzi February 26, 2023 03:46
@bbovenzi
Copy link
Contributor

bbovenzi commented Mar 1, 2023

Looking better!

  1. There are some code formatting errors to fix.
  2. We should add the sorting information to both places we use generate_pages, the other being audit_log in views.py
  3. Let's add a test to check_generate_pages_html that includes sorting key and direction

@amoghrajesh
Copy link
Contributor Author

Thank you for the review @bbovenzi
Trying to understand how to put the unit test in place. Will ping here if i have doubts

airflow/www/utils.py Outdated Show resolved Hide resolved
@eladkal eladkal added this to the Airflow 2.5.2 milestone Mar 2, 2023
@eladkal eladkal added the type:bug-fix Changelog: Bug Fixes label Mar 2, 2023
@bbovenzi bbovenzi merged commit c8cd90f into apache:main Mar 2, 2023
pierrejeambrun pushed a commit that referenced this pull request Mar 7, 2023
* Adding sorting keys to generate_pages

* Handling review comments and using the new keys

* Fixing Brent review comments

* Adding docstring

---------

Co-authored-by: Amogh <adesai@cloudera.com>
(cherry picked from commit c8cd90f)
pierrejeambrun pushed a commit that referenced this pull request Mar 8, 2023
* Adding sorting keys to generate_pages

* Handling review comments and using the new keys

* Fixing Brent review comments

* Adding docstring

---------

Co-authored-by: Amogh <adesai@cloudera.com>
(cherry picked from commit c8cd90f)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:webserver Webserver related Issues type:bug-fix Changelog: Bug Fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DAG list, sorting lost when switching page
5 participants