pierrejeambrun commented on code in PR #74764:
URL: https://github.com/apache/airflow/pull/74764#discussion_r4315163343
##########
airflow-core/src/airflow/ui/src/pages/TaskInstances/TaskInstancesFilter.tsx:
##########
@@ -58,6 +59,7 @@ export const TaskInstancesFilter = () => {
DAG_VERSION_PARAM as FilterableSearchParamsKeys,
OPERATOR_NAME_PATTERN_PARAM as FilterableSearchParamsKeys,
POOL_NAME_PATTERN_PARAM as FilterableSearchParamsKeys,
+ EXECUTOR_PARAM as FilterableSearchParamsKeys,
Review Comment:
When I tested this locally, the Executor pill shows "Executor:" with no
value, both from a URL and right after typing one. A `run_after_gte` link
filters the table without showing any pill. I think it's because the
`initialValues` loop below only reads single string values. `useFiltersHandler`
already returns `initialValues` that handle multiselect and date ranges, the
way Dag Runs and XCom use it. Could we switch to that? (The `useSearchParams`
call and the `FilterValue` import can then go too.)
```tsx
const { filterConfigs, handleFiltersChange, initialValues } =
useFiltersHandler(paramKeys);
```
##########
airflow-core/src/airflow/ui/src/constants/filterConfigs.tsx:
##########
@@ -222,6 +232,14 @@ export const useFilterConfigs = () => {
supportsAdvancedSearch: true,
type: FilterTypes.TEXT,
},
+ [SearchParamsKeys.EXECUTOR]: {
Review Comment:
If I'm reading this right, `executor` on a task instance is only set when
the task itself names one (`executor=...`). On a default deployment it's null
everywhere, so typing e.g. `CeleryExecutor` returns an empty table. I tried
that on a Celery setup. Should we drop this one for now, or make it clear it
only matches tasks that pin an executor?
##########
airflow-core/src/airflow/ui/src/pages/TaskInstances/TaskInstancesFilter.tsx:
##########
@@ -48,7 +49,7 @@ export const TaskInstancesFilter = () => {
const multiTeamEnabled = Boolean(useConfig("multi_team"));
const paramKeys: Array<FilterableSearchParamsKeys> = [
NAME_PATTERN_PARAM as FilterableSearchParamsKeys,
- LOGICAL_DATE_RANGE_PARAM as FilterableSearchParamsKeys,
+ RUN_AFTER_RANGE_PARAM as FilterableSearchParamsKeys,
Review Comment:
+1 to this. Could we keep Logical Date and add Run After next to it, like
Dag Runs and XCom do? Together with the `initialValues` change, existing links
would then show a pill that can be cleared.
```suggestion
LOGICAL_DATE_RANGE_PARAM as FilterableSearchParamsKeys,
RUN_AFTER_RANGE_PARAM as FilterableSearchParamsKeys,
```
--
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]
