Skip to content
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 auto refresh to dags home page #22900

Merged
merged 13 commits into from
May 1, 2022
Merged

Conversation

talnagar
Copy link
Contributor

Add auto refresh functionality to the home page (dags).
closes: #22418

refresh every 2 seconds:

  • last dag runs
  • last run
  • recent tasks

before:
image
after:
image

@boring-cyborg boring-cyborg bot added area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues labels Apr 11, 2022
@talnagar
Copy link
Contributor Author

@bbovenzi I would appreciate your feedback on this.
In addition, I didn't know whether there are any tests relevant for ui only changes such as this.
thanks.

@talnagar talnagar force-pushed the auto_refresh_dags branch from 8de20e3 to 93757f6 Compare April 11, 2022 13:07
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.

Looking great! A few comments

  • Let's avoid refreshing when the page isn't focused (see graph.js)
  • The formatting of the loading dots and auto-refresh text breaks on screens below 1380px in width. Let's change the .refresh-actions > .switch-label { in main.css to margin: 0 10px;
  • In grid and graph, we turn off refresh when there are no longer any active runs. I wonder if we should something similar here or at least increase the refresh interval.

@talnagar
Copy link
Contributor Author

@bbovenzi thanks for the review.
As suggested

  • paused the refresh when the page isn't focused
  • unchecked auto refresh when there are no more active runs
  • I changed the css and template layout a bit so it won't break, hopefully now it looks better on all screen sizes.

@bbovenzi
Copy link
Contributor

bbovenzi commented Apr 13, 2022

Let's still fix the margin for the switch. Even with removing that classname, this still happens:
Screen Shot 2022-04-13 at 2 04 20 PM

@talnagar
Copy link
Contributor Author

@bbovenzi for some reason I can't reproduce this break. I removed the margin now hopefully it looks ok.

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.

Great work! I think people will be really excited to have this.

But let's not merge until 2.3.0b1 is out

@talnagar
Copy link
Contributor Author

Great work! I think people will be really excited to have this.

But let's not merge until 2.3.0b1 is out

Thanks @bbovenzi! 😃

@github-actions github-actions bot added the okay to merge It's ok to merge this PR as it does not require more tests label Apr 14, 2022
@github-actions
Copy link

The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.

@talnagar
Copy link
Contributor Author

talnagar commented May 1, 2022

@bbovenzi can this be merged now?

@bbovenzi bbovenzi merged commit cd70afd into apache:main May 1, 2022
@ephraimbuddy ephraimbuddy added the type:new-feature Changelog: New Features label May 8, 2022
@ephraimbuddy ephraimbuddy added this to the Airflow 2.4.0 milestone May 8, 2022
@marclamberti
Copy link

OMG! this is AWESOME ❤️

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 okay to merge It's ok to merge this PR as it does not require more tests type:new-feature Changelog: New Features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

auto refresh Dags home page
4 participants