henry3260 commented on code in PR #61525:
URL: https://github.com/apache/airflow/pull/61525#discussion_r2774793240


##########
airflow-ctl/src/airflowctl/api/operations.py:
##########
@@ -554,20 +554,36 @@ def get(self, dag_id: str, dag_run_id: str) -> 
DAGRunResponse | ServerResponseEr
 
     def list(
         self,
-        dag_id: str,
-        start_date: datetime.datetime,
-        end_date: datetime.datetime,
-        state: str,
-        limit: int,
+        start_date: datetime.datetime | None = None,
+        end_date: datetime.datetime | None = None,
+        state: str | None = None,
+        limit: int | None = None,
+        dag_id: str | None = None,
     ) -> DAGRunCollectionResponse | ServerResponseError:
-        """List all dag runs."""
-        params = {
-            "start_date": start_date,
-            "end_date": end_date,
-            "state": state,
-            "limit": limit,
-            "dag_id": dag_id,
-        }
+        """
+        List all dag runs.
+
+        Args:
+            start_date: Filter dag runs by start date (optional)
+            end_date: Filter dag runs by end date (optional)
+            state: Filter dag runs by state (optional)
+            limit: Limit the number of results (optional)
+            dag_id: The DAG ID to filter by. If None, retrieves dag runs for 
all DAGs (using "~").
+        """
+        # Use "~" for all DAGs if dag_id is not specified
+        if not dag_id:
+            dag_id = "~"
+
+        params = {}
+        if start_date is not None:
+            params["start_date"] = start_date
+        if end_date is not None:
+            params["end_date"] = end_date
+        if state is not None:
+            params["state"] = state
+        if limit is not None:
+            params["limit"] = limit

Review Comment:
   > ## Could we implement this as -
   > ```
   >  if dag_id == None:
   >            dag_id = "~"
   >        params = {
   >            "start_date": start_date,
   >            "end_date": end_date,
   >            "state": state,
   >            "limit": limit,
   >            "dag_id": dag_id,
   >        }
   > ```
   > 
   > Could you please share your view ?
   
   Thanks for reviewing! IMO, I think using `if not dag_id:` is more robust, 
because it also handles empty string `("")` in addition to `None`. In this PR, 
we allow parameters like `state, limit, start_date, end_date` to be optional . 
When users don't provide value, they will be `None` and should not be included 
in the API request.



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to