AIP-84 | Add multisort to dags list request

[bbovenzi]

In the UI, we default the dags list to show dags with the latest dag run first , which we pass as -last_dag_run_start_date. Problem is that the minus symbol seems to make the default sorts also reverse so any dags without a dag run will be in reverse alphabetical order which is counterintuitive for users.
If you reverse the dag run start date order, it will also change the alphabetical order.

[pierrejeambrun]

Nice. For now the dag_id (name) is always used as the secondary sort criteria to ensure stability in the result set returned. (In case the primary criteria has identical values).

When we use a descending sort, the secondary filter (which is the primary key of the table, here the dag_id is also .desc()).

In the following code column is the primary sort criteria, the one specified by the user. primary_key_column is the secondary sort criteria always used and retrieved from the models.

airflow/airflow/api_fastapi/common/parameters.py

Lines 202 to 205 in ee6bd7e

	if self.value[0] == "-":
	return select.order_by(nullscheck, column.desc(), primary_key_column.desc())
	else:
	return select.order_by(nullscheck, column.asc(), primary_key_column.asc())

I think we need to support multisort to be able to handle this case. Users would be able to specify that they want to sort on the primary criteria in asc(), but on a secondary criteria in desc() order for intance. (Or any other arbitrary combination).

[Vishnu-sai-teja]

Hey can work on this.

[pierrejeambrun]

Great, assigned !

[prasad-madine]

Hello @pierrejeambrun, Are both front-end and back-end changes being done by the assignee?

[pierrejeambrun]

Hello @prasad-madine, I assume so.

We need the backend contribution to start working on the front-end anyway.

@Vishnu-sai-teja any progress ? Do you have plan on following up with the front-end changes as well ?

[Vishnu-sai-teja]

Hey out of town, haven't started on it yet, Sorry I should have informed earlier

[bbovenzi]

The UI part should be fairly straightforward: https://tanstack.com/table/latest/docs/guide/sorting#multi-sorting

[prasad-madine]

Hello @pierrejeambrun ,
I am currently working on implementing sorting functionality in the backend and would appreciate your guidance on the following two approaches:

Option 1

1. Allow user to select the primary sort field and order(asc/desc)
2. Allow user to select the secondary sort field and order(asc/desc)
3. Allow user to select which field is primary and which field is secondary

Option 2

1. Allow user to select the primary sort field and order(asc/desc)
2. Do not allow user to select the secondary sort field. Secondary sort field is always primary key.
3. Allow user to select the order in which we sort based on primary key.

Could you please advise on which option would be more appropriate or effective in this context?

[bbovenzi]

I would say option 1. The sort param would be passed in the url like so: order_by=-run-after&order_by=dag_display_name

The primary sort should be the first order_by param passed. Secondary sort, the second param.

[pierrejeambrun]

I would say option 1. The sort param would be passed in the url like so: order_by=-run-after&order_by=dag_display_name

The primary sort should be the first order_by param passed. Secondary sort, the second param.

That's also what I had in mind.

[prasad-madine]

Hello @bbovenzi @pierrejeambrun , I've implemented the backend code in below PR, Could you please review that.
#47440
Thankyou.

[mariana-marcal-santana]

Hello @bbovenzi @pierrejeambrun ! I'm a senior CS and engineering student and i would love to contribute to the frontend part of this issue for a project for one of my final classes. Would that be possible?

[pierrejeambrun]

@mariana-marcal-santana you can follow along the backend change #47440.

@prasad-madine were you planning on following up with the frontend once you are done with the backend or do you want @mariana-marcal-santana to take this work ?

[prasad-madine]

@mariana-marcal-santana you can follow along the backend change #47440.

@prasad-madine were you planning on following up with the frontend once you are done with the backend or do you want @mariana-marcal-santana to take this work ?

Yeah @mariana-marcal-santana can work on frontend portion.
Thanks

[pierrejeambrun]

@mariana-marcal-santana you'r also assigned

[mariana-marcal-santana]

Hello @bbovenzi @pierrejeambrun ! I've been working on the frontend of this issue and I believe I'm in the right track to solve it. However, I have a few things that I would like to confirm.
I followed the instructions on the link provided here #46383 (comment) to implement the multisort in the DAG table and now it's possible to define more sort parameters in the URL, as shown in the picture bellow.

As seen in the picture, the DAGs should be sorted firstly by "last_run_start_date" (OK) and secondly by "-dag_id" (not OK) because they appear sorted by "dag_id". Given this, I'm not sure where my implementation could be going wrong or if it's something related to the backend. @prasad-madine sorry to bother but could you take a look into this case please?
I also wanted to point out that the parameters are mentioned in the URL as "sort=" and not "order_by=" like mentioned here #46383 (comment). I'm not sure if that would be a problem, especially because since I've been working on this issue, they've always appeared as "sort=" in the UI but I would appreciate it if someone could confirm that.
Thank you!

[pierrejeambrun]

@mariana-marcal-santana Thanks, looking good.

#47859 (backend) has not been merged into main yet, are you building your PR on top of #47859 ? Otherwise you will need to wait for the backend to be completed (it is not supported yet).

[pierrejeambrun]

I'm not sure if that would be a problem, especially because since I've been working on this issue, they've always appeared as "sort=" in the UI but I would appreciate it if someone could confirm that.

Indeed, in the url we use sort and the front-end converts this to order_by before sending the request to the backend. (I don't know why we just didn't use orderBy too in the url, but just to answer your question, this is expected and shouldn't be a problem.)

[mariana-marcal-santana]

#47859 (backend) has not been merged into main yet, are you building your PR on top of #47859 ? Otherwise you will need to wait for the backend to be completed (it is not supported yet).

For the multisort to fully work it needs the #47440's backend but I tested locally my changes by themselves and they don't break the platform even without the corresponding backend. I don't mind waiting for the backend and if anything is needed I would be happy to help!

Also, maybe I didn't fully understand the question but I don't know much about the PR mentioned in the comment and if it has any effect on mine...

[mariana-marcal-santana]

Indeed, in the url we use sort and the front-end converts this to order_by before sending the request to the backend. (I don't know why we just didn't use orderBy too in the url, but just to answer your question, this is expected and shouldn't be a problem.)

Thanks for clearing that out!

[mariana-marcal-santana]

Hello @bbovenzi @pierrejeambrun !
Have you gotten a chance to look at the fixes from last commit?
Thanks

[pierrejeambrun]

Hello @mariana-marcal-santana,

Sorry for the delay, I just did a review, it will be easier to test and iterate once the backend is merged, we're working on that.