pierrejeambrun commented on code in PR #54049:
URL: https://github.com/apache/airflow/pull/54049#discussion_r2263617404


##########
airflow-core/src/airflow/api_fastapi/common/parameters.py:
##########
@@ -732,6 +733,20 @@ def _transform_ti_states(states: list[str] | None) -> 
list[TaskInstanceState | N
     ),
 ]
 
+# XCom
+QueryXComKeyPatternSearch = Annotated[
+    _SearchParam, Depends(search_param_factory(XComModel.key, 
"xcom_key_pattern"))
+]
+QueryXComDagIdPatternSearch = Annotated[
+    _SearchParam, Depends(search_param_factory(XComModel.dag_id, 
"dag_id_pattern"))
+]
+QueryXComRunIdPatternSearch = Annotated[
+    _SearchParam, Depends(search_param_factory(XComModel.run_id, 
"run_id_pattern"))
+]
+QueryXComTaskIdPatternSearch = Annotated[
+    _SearchParam, Depends(search_param_factory(XComModel.task_id, 
"task_id_pattern"))
+]

Review Comment:
   In the UI we most likely display `dag_display_name` but here we filter on 
`dag_id` which might confuse users.
   



##########
airflow-core/src/airflow/ui/src/pages/XCom/XCom.tsx:
##########
@@ -103,25 +112,49 @@ export const XCom = () => {
   const { t: translate } = useTranslation(["browse", "common"]);
   const { setTableURLState, tableURLState } = useTableURLState();
   const { pagination } = tableURLState;
+  const [searchParams] = useSearchParams();
 
-  const { data, error, isFetching, isLoading } = useXcomServiceGetXcomEntries(
-    {
-      dagId,
-      dagRunId: runId,
-      limit: pagination.pageSize,
-      mapIndex: mapIndex === "-1" ? undefined : parseInt(mapIndex, 10),
-      offset: pagination.pageIndex * pagination.pageSize,
-      taskId,
-    },
-    undefined,
-    { enabled: !isNaN(pagination.pageSize) },
-  );
+  const filteredKey = searchParams.get(KEY_PATTERN_PARAM);
+  const filteredDagId = searchParams.get(DAG_ID_PATTERN_PARAM);
+  const filteredRunId = searchParams.get(RUN_ID_PATTERN_PARAM);
+  const filteredTaskId = searchParams.get(TASK_ID_PATTERN_PARAM);
+
+  const { LOGICAL_DATE_GTE, LOGICAL_DATE_LTE, RUN_AFTER_GTE, RUN_AFTER_LTE } = 
SearchParamsKeys;
+
+  const logicalDateGte = searchParams.get(LOGICAL_DATE_GTE);
+  const logicalDateLte = searchParams.get(LOGICAL_DATE_LTE);
+  const runAfterGte = searchParams.get(RUN_AFTER_GTE);
+  const runAfterLte = searchParams.get(RUN_AFTER_LTE);
+
+  const apiParams = {
+    dagId,
+    dagIdPattern: filteredDagId ?? undefined,
+    dagRunId: runId,
+    limit: pagination.pageSize,
+    logicalDateGte: logicalDateGte ?? undefined,
+    logicalDateLte: logicalDateLte ?? undefined,
+    mapIndex: mapIndex === "-1" ? undefined : parseInt(mapIndex, 10),
+    offset: pagination.pageIndex * pagination.pageSize,
+    runAfterGte: runAfterGte ?? undefined,
+    runAfterLte: runAfterLte ?? undefined,
+    runIdPattern: filteredRunId ?? undefined,
+    taskId,
+    taskIdPattern: filteredTaskId ?? undefined,
+    xcomKeyPattern: filteredKey ?? undefined,
+  };
+
+  const { data, error, isFetching, isLoading } = 
useXcomServiceGetXcomEntries(apiParams, undefined, {
+    enabled: !isNaN(pagination.pageSize),
+  });
 
   return (
     <Box>
       {dagId === "~" && runId === "~" && taskId === "~" ? (
         <Heading size="md">{translate("xcom.title")}</Heading>
       ) : undefined}
+
+      {dagId === "~" && runId === "~" && taskId === "~" ? <XComFilters /> : 
undefined}
+

Review Comment:
   It would be nice to still have other filtering option on details pages.



##########
airflow-core/src/airflow/ui/public/i18n/locales/en/browse.json:
##########
@@ -18,6 +18,16 @@
       "key": "Key",
       "value": "Value"
     },
+    "filters": {
+      "dagFilter": "Filter by Dag ID",
+      "keyFilter": "Filter by XCom key",
+      "logicalDateFrom": "Logical Date From",
+      "logicalDateTo": "Logical Date To",
+      "runAfterFrom": "Run After From",
+      "runAfterTo": "Run After To",
+      "runIdFilter": "Filter by Run ID",
+      "taskIdFilter": "Filter by Task ID"
+    },

Review Comment:
   We most likely have some of those key already existing, we might want to 
regroup them. key with "Placeholder" suffix. Those should also have 
"Placeholder" suffix.



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