Skip to content

Remove last usages of grid data and remove grid_data endpoint. #52672

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 13 commits into from
Jul 9, 2025
Merged

Remove last usages of grid data and remove grid_data endpoint. #52672

merged 13 commits into from
Jul 9, 2025
+165 −2,258

Conversation

dstandish
Copy link
Contributor

No description provided.

@boring-cyborg boring-cyborg bot added area:API Airflow's REST/HTTP API area:UI Related to UI/UX. For Frontend Developers. labels Jul 1, 2025
@dstandish dstandish added the backport-to-v3-0-test Mark PR with this label to backport to v3-0-test branch label Jul 1, 2025
@dstandish dstandish added this to the Airflow 3.0.3 milestone Jul 1, 2025
@dstandish dstandish changed the title Remove more usages of grid data and remove grid_data endpoint. Remove last usages of grid data and remove grid_data endpoint. Jul 1, 2025
@uranusjr uranusjr modified the milestones: Airflow 3.0.3, Airflow 3.1.0 Jul 2, 2025
@uranusjr uranusjr removed the backport-to-v3-0-test Mark PR with this label to backport to v3-0-test branch label Jul 2, 2025
@uranusjr
Copy link
Member

uranusjr commented Jul 2, 2025

Probably don’t need to backport this since it’s fundamentally refactoring.

bugraoz93
bugraoz93 reviewed Jul 2, 2025
View reviewed changes
Copy link
Contributor

@bugraoz93 bugraoz93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still need to clean some tests and fix static checks. Overall, it looks deleting all other parts

pierrejeambrun
pierrejeambrun reviewed Jul 3, 2025
View reviewed changes
Copy link
Member

@pierrejeambrun pierrejeambrun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️ can't wait for this PR to be green, removing all that code

@bugraoz93
Copy link
Contributor

Huge thanks for all the work on this, Daniel! I am genuinely sorry again for creating that monolithic code with its repeated, full TI aggregations. I know it created a lot of extra work for a couple of people and caused scaling problems. I truly appreciate that you have been taking the time to simplify and improve it.

@dstandish dstandish force-pushed the remove-more-usages-of-grid-data branch from 1c9f7b9 to a24cf13 Compare July 3, 2025 17:43
@kaxil kaxil mentioned this pull request Jul 3, 2025
Merged
pierrejeambrun
pierrejeambrun approved these changes Jul 4, 2025
View reviewed changes
Copy link
Member

@pierrejeambrun pierrejeambrun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some ESlint issues that need fixing I believe.

@pierrejeambrun pierrejeambrun mentioned this pull request Jul 5, 2025
Closed
@dstandish dstandish force-pushed the remove-more-usages-of-grid-data branch 2 times, most recently from 0948eb1 to d93d84c Compare July 8, 2025 11:51
@dstandish
Copy link
Contributor Author

i pointed out in the comments where i needed to simplify some things in tooltips because we don't have all of the same data info

@dstandish dstandish force-pushed the remove-more-usages-of-grid-data branch from d93d84c to 954e31b Compare July 8, 2025 16:10
@dstandish
Copy link
Contributor Author

ok i added back the bit where we need the "child count" to show mapped task count

here you can see that, and you can see that the tooltip is there (though it just shows the TI state here

in other usages of TaskInstanceTooltip, we are getting the data from elsewhere and there is more info

image

@bbovenzi
Copy link
Contributor

bbovenzi commented Jul 8, 2025

ok i added back the bit where we need the "child count" to show mapped task count

here you can see that, and you can see that the tooltip is there (though it just shows the TI state here

in other usages of TaskInstanceTooltip, we are getting the data from elsewhere and there is more info

image

If its just the state then feel free to comment out the tooltip for now. Not a blocker to this PR though

@bbovenzi
Copy link
Contributor

bbovenzi commented Jul 8, 2025

Merge conflicts

@dstandish dstandish force-pushed the remove-more-usages-of-grid-data branch from 9852544 to b044c04 Compare July 8, 2025 21:52
@dstandish
Copy link
Contributor Author

Merge conflicts

thanks, rebased

@bbovenzi
Copy link
Contributor

bbovenzi commented Jul 9, 2025

Another day another merge conflict. We merge other things too quickly!

@dstandish
Copy link
Contributor Author

Another day another merge conflict. We merge other things too quickly!

oh boy

@dstandish dstandish force-pushed the remove-more-usages-of-grid-data branch from 1861753 to e984ce5 Compare July 9, 2025 16:17
@dstandish dstandish merged commit 7ee6526 into apache:main Jul 9, 2025
102 checks passed
@dstandish dstandish deleted the remove-more-usages-of-grid-data branch July 9, 2025 17:48
HsiuChuanHsu pushed a commit to HsiuChuanHsu/airflow that referenced this pull request Jul 10, 2025
@dstandish @HsiuChuanHsu
Remove last usages of grid data and remove grid_data endpoint. (apach…
f05c90e 
…e#52672)

Get rid of the old grid_data endpoint.  It was already replaced in the grid view.  But there were still a few uses in the graph view, namely for tooltips.
stephen-bracken pushed a commit to stephen-bracken/airflow that referenced this pull request Jul 15, 2025
@dstandish
Remove last usages of grid data and remove grid_data endpoint. (apach…
533a930 
…e#52672)

Get rid of the old grid_data endpoint.  It was already replaced in the grid view.  But there were still a few uses in the graph view, namely for tooltips.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bbovenzi bbovenzi bbovenzi approved these changes

@bugraoz93 bugraoz93 bugraoz93 approved these changes

@pierrejeambrun pierrejeambrun pierrejeambrun approved these changes

@ryanahamilton ryanahamilton Awaiting requested review from ryanahamilton ryanahamilton is a code owner

@jscheffl jscheffl Awaiting requested review from jscheffl jscheffl is a code owner

@shubhamraj-git shubhamraj-git Awaiting requested review from shubhamraj-git shubhamraj-git is a code owner

@ephraimbuddy ephraimbuddy Awaiting requested review from ephraimbuddy ephraimbuddy is a code owner

@rawwar rawwar Awaiting requested review from rawwar rawwar is a code owner

@jason810496 jason810496 Awaiting requested review from jason810496 jason810496 is a code owner

Assignees
No one assigned
Labels
area:API Airflow's REST/HTTP API area:UI Related to UI/UX. For Frontend Developers.
Projects
None yet
Milestone
Airflow 3.1.0
Development

Successfully merging this pull request may close these issues.
5 participants
@dstandish @uranusjr @bugraoz93 @bbovenzi @pierrejeambrun