Skip to content

Fix auto refresh for graph view#26926

Merged
bbovenzi merged 3 commits intoapache:mainfrom
pierrejeambrun:fix-graph-auto-refresh
Oct 11, 2022
Merged

Fix auto refresh for graph view#26926
bbovenzi merged 3 commits intoapache:mainfrom
pierrejeambrun:fix-graph-auto-refresh

Conversation

@pierrejeambrun
Copy link
Member

@pierrejeambrun pierrejeambrun commented Oct 6, 2022

solves: #26901

#26554 introduced a proper way to set the json_provider_class. As a result the /object/task_instances endpoint now returns a json/application content type instead of text/html which is already parsed in the front. Explicitly calling JSON.parse is not necessary anymore handleRefresh.

Old Response:
image

Current response:
image

I figured adding a test for the /object/task_instances view wouldn't hurt.

@boring-cyborg boring-cyborg bot added area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues labels Oct 6, 2022
Copy link
Member Author

Choose a reason for hiding this comment

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

Is there a way to mock the server_default, for updated_at so I can assert the entire response ?

Copy link
Member

Choose a reason for hiding this comment

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

I pushed a commit to do this.

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks @uranusjr, much cleaner this way. I naively froze time on the test, which had no effect since fixture init happens before 🤦‍♂️

@pierrejeambrun pierrejeambrun force-pushed the fix-graph-auto-refresh branch from ec79721 to 8e4bc93 Compare October 7, 2022 16:39
@bbovenzi bbovenzi added this to the Airflow 2.4.2 milestone Oct 10, 2022
@@ -434,11 +434,10 @@ function handleRefresh() {
// only refresh if the data has changed
if (prevTis !== tis) {
// eslint-disable-next-line no-global-assign
Copy link
Contributor

Choose a reason for hiding this comment

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

We can get rid of this line now

Copy link
Contributor

@bbovenzi bbovenzi left a comment

Choose a reason for hiding this comment

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

not sure about the updated_at mock but this looks good to me.

@bbovenzi bbovenzi merged commit 6462292 into apache:main Oct 11, 2022
@pierrejeambrun pierrejeambrun deleted the fix-graph-auto-refresh branch October 11, 2022 15:59
@ephraimbuddy ephraimbuddy added the type:bug-fix Changelog: Bug Fixes label Oct 18, 2022
ephraimbuddy pushed a commit that referenced this pull request Oct 18, 2022
* Fix auto refresh for graph view

* Add task_instances view test

* Use freezegun to mock datetime

Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
(cherry picked from commit 6462292)
ephraimbuddy pushed a commit that referenced this pull request Oct 18, 2022
* Fix auto refresh for graph view

* Add task_instances view test

* Use freezegun to mock datetime

Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
(cherry picked from commit 6462292)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues type:bug-fix Changelog: Bug Fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants