From 69ac592d851a9b29c079c170d2974bc2ea5b4861 Mon Sep 17 00:00:00 2001 From: Victor Chiapaikeo Date: Tue, 28 Nov 2023 23:04:39 -0500 Subject: [PATCH] Use arrays instead of serializing to csv --- airflow/www/static/js/api/useGridData.ts | 1 + airflow/www/static/js/dag/nav/FilterBar.tsx | 16 ++--- airflow/www/static/js/dag/useFilters.test.tsx | 12 ++-- airflow/www/static/js/dag/useFilters.tsx | 59 ++++++++++--------- airflow/www/views.py | 10 ++-- tests/www/views/test_views_grid.py | 16 +++-- 6 files changed, 63 insertions(+), 51 deletions(-) diff --git a/airflow/www/static/js/api/useGridData.ts b/airflow/www/static/js/api/useGridData.ts index a0cbfd6b76555..502877214c83b 100644 --- a/airflow/www/static/js/api/useGridData.ts +++ b/airflow/www/static/js/api/useGridData.ts @@ -105,6 +105,7 @@ const useGridData = () => { }; const response = await axios.get(gridDataUrl, { params, + paramsSerializer: { indexes: null }, }); // turn off auto refresh if there are no active runs if (!areActiveRuns(response.dagRuns)) stopRefresh(); diff --git a/airflow/www/static/js/dag/nav/FilterBar.tsx b/airflow/www/static/js/dag/nav/FilterBar.tsx index dc3136ab21fa4..183f60e357d8c 100644 --- a/airflow/www/static/js/dag/nav/FilterBar.tsx +++ b/airflow/www/static/js/dag/nav/FilterBar.tsx @@ -46,7 +46,7 @@ const FilterBar = () => { onRunTypeChange, onRunStateChange, clearFilters, - transformCsvToMultiSelectOptions, + transformArrayToMultiSelectOptions, } = useFilters(); const { timezone } = useTimezone(); @@ -109,18 +109,18 @@ const FilterBar = () => { { if ( Array.isArray(typeOptions) && typeOptions.every((typeOption) => "value" in typeOption) ) { onRunTypeChange( - typeOptions.map((typeOption) => typeOption.value).join(",") + typeOptions.map((typeOption) => typeOption.value) ); } }} - options={transformCsvToMultiSelectOptions(filters.runTypeOptions)} + options={transformArrayToMultiSelectOptions(filters.runTypeOptions)} placeholder="All Run Types" /> @@ -128,18 +128,20 @@ const FilterBar = () => { { if ( Array.isArray(stateOptions) && stateOptions.every((stateOption) => "value" in stateOption) ) { onRunStateChange( - stateOptions.map((stateOption) => stateOption.value).join(",") + stateOptions.map((stateOption) => stateOption.value) ); } }} - options={transformCsvToMultiSelectOptions(filters.runStateOptions)} + options={transformArrayToMultiSelectOptions( + filters.runStateOptions + )} placeholder="All Run States" /> diff --git a/airflow/www/static/js/dag/useFilters.test.tsx b/airflow/www/static/js/dag/useFilters.test.tsx index 5ecb7fb88785c..72e1b42543e44 100644 --- a/airflow/www/static/js/dag/useFilters.test.tsx +++ b/airflow/www/static/js/dag/useFilters.test.tsx @@ -89,22 +89,22 @@ describe("Test useFilters hook", () => { { fnName: "onRunTypeChange" as keyof UtilFunctions, paramName: "runType" as keyof Filters, - paramValue: "manual", + paramValue: ["manual"], }, { fnName: "onRunTypeChange" as keyof UtilFunctions, paramName: "runType" as keyof Filters, - paramValue: "manual,backfill", + paramValue: ["manual", "backfill"], }, { fnName: "onRunStateChange" as keyof UtilFunctions, paramName: "runState" as keyof Filters, - paramValue: "success", + paramValue: ["success"], }, { fnName: "onRunStateChange" as keyof UtilFunctions, paramName: "runState" as keyof Filters, - paramValue: "success,failed,queued", + paramValue: ["success", "failed", "queued"], }, ])("Test $fnName functions", async ({ fnName, paramName, paramValue }) => { const { result } = renderHook( @@ -113,7 +113,9 @@ describe("Test useFilters hook", () => { ); await act(async () => { - result.current[fnName](paramValue as "string" & FilterTasksProps); + result.current[fnName]( + paramValue as "string" & string[] & FilterTasksProps + ); }); expect(result.current.filters[paramName]).toBe(paramValue); diff --git a/airflow/www/static/js/dag/useFilters.tsx b/airflow/www/static/js/dag/useFilters.tsx index 5136456c13e3d..2d5d2eb321d1d 100644 --- a/airflow/www/static/js/dag/useFilters.tsx +++ b/airflow/www/static/js/dag/useFilters.tsx @@ -38,10 +38,10 @@ export interface Filters { filterDownstream: boolean | undefined; baseDate: string | null; numRuns: string | null; - runType: string | null; - runTypeOptions: string | null; - runState: string | null; - runStateOptions: string | null; + runType: string[] | null; + runTypeOptions: string[] | null; + runState: string[] | null; + runStateOptions: string[] | null; } export interface FilterTasksProps { @@ -53,11 +53,11 @@ export interface FilterTasksProps { export interface UtilFunctions { onBaseDateChange: (value: string) => void; onNumRunsChange: (value: string) => void; - onRunTypeChange: (value: string) => void; - onRunStateChange: (value: string) => void; + onRunTypeChange: (values: string[]) => void; + onRunStateChange: (values: string[]) => void; onFilterTasksChange: (args: FilterTasksProps) => void; - transformCsvToMultiSelectOptions: ( - options: string | null + transformArrayToMultiSelectOptions: ( + options: string[] | null ) => { label: string; value: string }[]; clearFilters: () => void; resetRoot: () => void; @@ -97,14 +97,14 @@ const useFilters = (): FilterHookReturn => { const numRuns = searchParams.get(NUM_RUNS_PARAM) || defaultDagRunDisplayNumber.toString(); - const runTypeOptions = filtersOptions.runTypes.join(","); - const runType = searchParams.get(RUN_TYPE_PARAM); + const runTypeOptions = filtersOptions.runTypes; + const runType = searchParams.getAll(RUN_TYPE_PARAM); - const runStateOptions = filtersOptions.dagStates.join(","); - const runState = searchParams.get(RUN_STATE_PARAM); + const runStateOptions = filtersOptions.dagStates; + const runState = searchParams.getAll(RUN_STATE_PARAM); const makeOnChangeFn = - (paramName: string, formatFn?: (arg: string) => string | null) => + (paramName: string, formatFn?: (arg: string) => string) => (value: string) => { const formattedValue = formatFn ? formatFn(value) : value; const params = new URLSearchParamsWrapper(searchParams); @@ -115,20 +115,25 @@ const useFilters = (): FilterHookReturn => { setSearchParams(params); }; - const getMultiSelectFormatFn = (options: string[]) => { - const formatFn = (arg: string) => { - const argLength = arg.split(",").length; - return argLength === options.length || argLength === 0 ? null : arg; + const makeMultiSelectOnChangeFn = + (paramName: string, options: string[]) => (values: string[]) => { + const params = new URLSearchParamsWrapper(searchParams); + if (values.length === options.length || values.length === 0) { + params.delete(paramName); + } else { + // Delete and reinsert anew each time; otherwise, there will be duplicates + params.delete(paramName); + values.forEach((value) => params.append(paramName, value)); + } + setSearchParams(params); }; - return formatFn; - }; - const transformCsvToMultiSelectOptions = ( - options: string | null + const transformArrayToMultiSelectOptions = ( + options: string[] | null ): { label: string; value: string }[] => options === null ? [] - : options.split(",").map((option) => ({ label: option, value: option })); + : options.map((option) => ({ label: option, value: option })); const onBaseDateChange = makeOnChangeFn( BASE_DATE_PARAM, @@ -136,13 +141,13 @@ const useFilters = (): FilterHookReturn => { (localDate: string) => moment(localDate).utc().format() ); const onNumRunsChange = makeOnChangeFn(NUM_RUNS_PARAM); - const onRunTypeChange = makeOnChangeFn( + const onRunTypeChange = makeMultiSelectOnChangeFn( RUN_TYPE_PARAM, - getMultiSelectFormatFn(filtersOptions.runTypes) + filtersOptions.runTypes ); - const onRunStateChange = makeOnChangeFn( + const onRunStateChange = makeMultiSelectOnChangeFn( RUN_STATE_PARAM, - getMultiSelectFormatFn(filtersOptions.dagStates) + filtersOptions.dagStates ); const onFilterTasksChange = ({ @@ -203,7 +208,7 @@ const useFilters = (): FilterHookReturn => { onFilterTasksChange, clearFilters, resetRoot, - transformCsvToMultiSelectOptions, + transformArrayToMultiSelectOptions, }; }; diff --git a/airflow/www/views.py b/airflow/www/views.py index eb6bc033c2933..649440865c35c 100644 --- a/airflow/www/views.py +++ b/airflow/www/views.py @@ -3527,14 +3527,12 @@ def grid_data(self): with create_session() as session: query = select(DagRun).where(DagRun.dag_id == dag.dag_id, DagRun.execution_date <= base_date) - run_type_raw = request.args.get("run_type") - if run_type_raw: - run_types = {run_type.strip() for run_type in run_type_raw.split(",")} + run_types = request.args.getlist("run_type") + if run_types: query = query.where(DagRun.run_type.in_(run_types)) - run_state_raw = request.args.get("run_state") - if run_state_raw: - run_states = {run_state.strip() for run_state in run_state_raw.split(",")} + run_states = request.args.getlist("run_state") + if run_states: query = query.where(DagRun.state.in_(run_states)) dag_runs = wwwutils.sorted_dag_runs( diff --git a/tests/www/views/test_views_grid.py b/tests/www/views/test_views_grid.py index 3b5fb46999de1..0e6e92b23c843 100644 --- a/tests/www/views/test_views_grid.py +++ b/tests/www/views/test_views_grid.py @@ -164,12 +164,16 @@ def test_no_runs(admin_client, dag_without_runs): def test_grid_data_filtered_on_run_type_and_run_state(admin_client, dag_with_runs): for uri_params, expected_run_types, expected_run_states in [ - ("run_state=success%2Cqueued", ["scheduled"], ["success"]), - ("run_state=running%2Cfailed", ["scheduled"], ["running"]), - ("run_type=scheduled%2Cmanual", ["scheduled", "scheduled"], ["success", "running"]), - ("run_type=backfill%2Cmanual", [], []), - ("run_state=running%2Cfailed&run_type=backfill%2Cmanual", [], []), - ("run_state=running%2Cfailed&run_type=scheduled%2Cbackfill%2Cmanual", ["scheduled"], ["running"]), + ("run_state=success&run_state=queued", ["scheduled"], ["success"]), + ("run_state=running&run_state=failed", ["scheduled"], ["running"]), + ("run_type=scheduled&run_type=manual", ["scheduled", "scheduled"], ["success", "running"]), + ("run_type=backfill&run_type=manual", [], []), + ("run_state=running&run_type=failed&run_type=backfill&run_type=manual", [], []), + ( + "run_state=running&run_type=failed&run_type=scheduled&run_type=backfill&run_type=manual", + ["scheduled"], + ["running"], + ), ]: resp = admin_client.get(f"/object/grid_data?dag_id={DAG_ID}&{uri_params}", follow_redirects=True) assert resp.status_code == 200, resp.json