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



##########
File path: airflow/www/utils.py
##########
@@ -302,6 +311,48 @@ def dag_run_link(attr):
         '<a href="{url}">{run_id}</a>').format(url=url, run_id=run_id)  # noqa
 
 
+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,
+        # <- add any extra (URL param)->(DagModel attr) mappings here
+    }

Review comment:
       It might be easier/less code to have a single `sort_by` and have this as
   
   ```suggestion
       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.
   
   Additionally this order probably doesn't belong in this file -- but instead 
in the views.py file, probably in the `home` function.




----------------------------------------------------------------
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:
[email protected]


Reply via email to