[GitHub] [airflow] ashb commented on a change in pull request #11652: Support for sorting DAGs in the web UI

[GitBox <gi...@apache.org>]

ashb commented on a change in pull request #11652:
URL: https://github.com/apache/airflow/pull/11652#discussion_r535266580



##########
File path: airflow/www/views.py
##########
@@ -449,6 +490,8 @@ def get_int_arg(value, default=0):
         arg_search_query = request.args.get('search')
         arg_tags_filter = request.args.getlist('tags')
         arg_status_filter = request.args.get('status')
+        arg_sorting_key = request.args.get('sortBy')
+        arg_orderby_key = request.args.get('orderBy')

Review comment:
       These two params are two confusing/similar, and clash a bit with standard SQL meanings, since the sortBy value is the term after the `ORDER BY`.
   
   Could we instead have `orderBy` and `orderDir` (direction).
   
   (I also would have personally done it as `orderBy=dag_id` for asc and `orderBy=-dag_id` for desc. It means you can then order by multiple columns with `orderBy=col_a,-col_b` or `orderBy=col_a&orderBy=-col_b`. But feel free to leave it as two params, it's fine as it is)

##########
File path: airflow/www/views.py
##########
@@ -332,6 +332,47 @@ def get_downstream(task):
     ]
 
 
+def dag_query_for_key(sorting_key):
+    """Maps sorting key param in the URL params to DB queries on appropriate attributes."""
+    dag_query_key_map = {
+        'dag_id': DagModel.dag_id,
+        'owner': DagModel.owners,
+        'schedule': DagModel.next_dagrun,

Review comment:
       I think I'd rather these values matched the column names -- we're already having to provide the "Readable String" and the order by in the template.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org