Add run_id filter for dag runs #52437
Conversation
|
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
|
There was a problem hiding this comment.
I am not fully sure about this feature, it is really a gap from previous version but the implementation only offers an exact match. No globbing.
I'd request some other reviewers as well, I assume next level is also we need to invest into better search capabilities in general, only exact run ID match is not much of a great value.
Please do not treat it tooo negative. I think in general the search capabilties are a shortcoming in the UI in general. I am unsure about the increments needed to get back to 2.11 state of options.
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dag_run.py
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dag_run.py
Outdated
Show resolved
Hide resolved
|
That was fast! Awesome work. There are a number of other fields that could be sorted on, and I agree with
However, I do also think something is better than nothing? |
There was a problem hiding this comment.
Thanks for the pull request, that's a good idea.
We do support searching on patterns:
DESCRIPTION = (
"SQL LIKE expression — use `%` / `_` wildcards (e.g. `%customer_%`). "
"Regular expressions are **not** supported."
)
You can take a look at search_param_factory to construct such filters on the API side.
For instance for the task_display_name_pattern search.
| @@ -334,6 +336,7 @@ def get_dag_runs( | |||
| readable_dag_runs_filter: ReadableDagRunsFilterDep, | |||
| session: SessionDep, | |||
| dag_bag: DagBagDep, | |||
| run_id: Annotated[_SearchParam, Depends(search_param_factory(DagRun.run_id, "run_id"))], | |||
There was a problem hiding this comment.
run_id_pattern to match other search query param in the API.
| @@ -26,6 +26,7 @@ export enum SearchParamsKeys { | |||
| OFFSET = "offset", | |||
| PAUSED = "paused", | |||
| POOL = "pool", | |||
| RUN_ID = "run_id", | |||
There was a problem hiding this comment.
RUN_ID_PATTERN="run_id_pattern"
| <Input | ||
| maxW="200px" | ||
| onChange={handleRunIdChange} | ||
| placeholder={translate("dags:filters.runIdFilter")} |
There was a problem hiding this comment.
| placeholder={translate("dags:filters.runIdFilter")} | |
| placeholder={translate("dags:filters.runIdPatternFilter")} |
Really important remarks, Just applied to support pattern search. Thanks! I have some questions:
|
| <InputGroup startElement={<FiHash size={14} />}> | ||
| <Input | ||
| maxW="200px" | ||
| onChange={handleRunIdChange} | ||
| placeholder={translate("dags:filters.runIdFilter")} | ||
| value={filteredRunId ?? ""} | ||
| /> | ||
| </InputGroup> |
There was a problem hiding this comment.
Why don't we use the already existing SearchBar.tsx component? Especially if we update to to filter by run_id_pattern?
Feature
run_idfilter to/dag_runspageDiscussion points
run_idfilter sufficient? or should we support more complex search parser?Feedback and suggestions are welcome!
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.