-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Add filters to Task Instances Tab #56920
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add filters to Task Instances Tab #56920
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)
|
5e1a3a6 to
766ce8e
Compare
|
Implenting multiselect filter for |
766ce8e to
a7a2333
Compare
a7a2333 to
d019aa8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking this on. I think we can break this into smaller PRs.
First to move all filters into FilterBar and quickly add any simple filters that already exist in filterConfigs.tsx.
Second, to add new filter configs.
airflow-core/src/airflow/ui/public/i18n/locales/en/taskInstances.json
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/ui/src/pages/TaskInstances/AttrSelectFilterMulti.tsx
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/ui/src/pages/TaskInstances/TaskInstancesFilter.tsx
Show resolved
Hide resolved
airflow-core/src/airflow/ui/src/pages/TaskInstances/TaskInstancesFilter.tsx
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/ui/src/pages/TaskInstances/TaskInstancesFilter.tsx
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/ui/src/pages/TaskInstances/TaskInstancesFilter.tsx
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/ui/src/pages/TaskInstances/AttrSelectFilterMulti.tsx
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/ui/src/pages/TaskInstances/AttrSelectFilterMulti.tsx
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/ui/src/pages/TaskInstances/StateFilter.tsx
Outdated
Show resolved
Hide resolved
|
Hi @bbovenzi — thanks again for the review! I’ve addressed all requested changes. I’ve integrated state / operator / pool / queue into FilterBar (see UPDATE + screenshot). Follow-up: while testing locally with prek, ESLint reports: Would you prefer I refactor that in this PR, or open a separate PR to keep this one focused on the original issue? First time contributing to this repo, so I want to follow the preferred workflow. I can do either. Let me know your preference or any suggestions. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR.
Looks nice overall. A few things to address before we can merge.
The main problem being that we show 'search bar' for a few different filter where those filters are actually not 'search' in the backend but plain 'filter'. This means that until you fully typed the correct name in the search bar, nothing will be returned.
We can either convert UI filters to not use the searchbar and populate dropdowns somehow, or implement the search / pattern match counterpart filers in the backend for pool, queue etc... I think we should go with the backend filter solution, that could be a very simple separate PR. Or I can do it if you want, just let me know.
airflow-core/src/airflow/ui/src/pages/TaskInstances/TaskInstances.tsx
Outdated
Show resolved
Hide resolved
|
Hi @pierrejeambrun , Thanks for the review! |
d46cb59 to
ec051c6
Compare
UPDATE:
FollowUp:
|
ec051c6 to
8c0383d
Compare
RoyLee1224
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks great!
Just one nit about translation💪
8c0383d to
8e55291
Compare
guan404ming
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good in my local test
|
@RoyLee1224 could you help take another look? Thanks! |
Make sense to me. |
8e55291 to
256f877
Compare
|
Hi @guan404ming, @RoyLee1224, |
|
I think let's just integrate the fix into this PR since this one is not merged, what do you think? |
|
No problem! I'll integrated it very soon |
256f877 to
0ab672a
Compare
91a96a2 to
be78503
Compare
|
Just merged the date-range filter if we want to use it here |
32a366a to
8084ffb
Compare
|
Hi @bbovenzi, thanks for the review. Really appreciate your help!, I just merge the date range picker |
pierrejeambrun
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks
* Add several filters to Task View (squashed, de-duplicated) * fix traslation problem stick to Operator in task * fix format * fix too long file * fix format * add run_id_pattern as backend filter * fix format * change run_id filter to name pattern filter * merge date range picker
* Add several filters to Task View (squashed, de-duplicated) * fix traslation problem stick to Operator in task * fix format * fix too long file * fix format * add run_id_pattern as backend filter * fix format * change run_id filter to name pattern filter * merge date range picker

Related issues
closes: #53051
TODO:
Filters:
API:
already done by [#53042]
Screenshots
since filter for Task Instances at Task View and Search View are the same one, some demo clip is record on Search View for larger data set, showing the fuctionality
conf.mp4
^ 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.