refactor: move task stream filtering logic to endpoint for Grid#58576
Conversation
2ca6130 to
fed6a07
Compare
pierrejeambrun
left a comment
There was a problem hiding this comment.
Nice, thanks for the pull request. 🎉
Do you mind updating the tests for the backend endpoint (structure), for these newly added query parameters? (with a few different combinations)
92c71db to
cdfa245
Compare
Great point 👍 Adding the test actually showed me that I forgot to account for the retrieval of historical tasks. To make the code a bit clearer, I moved that part to I thought it would make sense to refrain from retrieving historical tasks in case of filtering the DAG. Let me know whether you agree or not! |
32b6a76 to
0295804
Compare
pierrejeambrun
left a comment
There was a problem hiding this comment.
Looking good.
Just one question to clarify before we can merge.
airflow-core/src/airflow/api_fastapi/core_api/routes/ui/grid.py
Outdated
Show resolved
Hide resolved
9f13179 to
5efe698
Compare
5efe698 to
2752bfb
Compare
|
Thanks @OscarLigthart! 🎉 |
…he#58576) * refactor: move task stream filtering logic to endpoint for Grid * fix: lint * feat: add tests for stream filter in grid * fix: historical task filtering * fix: return value for collect_historical_tasks * fix: remove duplicate historical task retrieval and add historical task to test case
…he#58576) * refactor: move task stream filtering logic to endpoint for Grid * fix: lint * feat: add tests for stream filter in grid * fix: historical task filtering * fix: return value for collect_historical_tasks * fix: remove duplicate historical task retrieval and add historical task to test case
…he#58576) * refactor: move task stream filtering logic to endpoint for Grid * fix: lint * feat: add tests for stream filter in grid * fix: historical task filtering * fix: return value for collect_historical_tasks * fix: remove duplicate historical task retrieval and add historical task to test case
Following PR #57237, @bbovenzi gave me a piece of feedback that we should try to avoid fetching all of the tasks a second time and then iterating through them all again on the frontend. Instead, we should add the filter parameters to the existing grid structure endpoint. Keeping scopes of PRs small, I decided to do it in separate PR.
Enter this PR, which introduces exactly that. It simplifies the frontend Grid behaviour in favour of implementing the filter logic in the backend.
^ 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.