-
Notifications
You must be signed in to change notification settings - Fork 16.3k
UI: Update PoolBar to separate Scheduled and Deferred slots #59270
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
UI: Update PoolBar to separate Scheduled and Deferred slots #59270
Conversation
airflow-core/src/airflow/ui/src/pages/Dashboard/PoolSummary/PoolSummary.tsx
Outdated
Show resolved
Hide resolved
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.
Also I think we need to implement a case for include_deferred.
When include_deferred is 'true' for the pool, deferred state will actually take an open slot, and in this case, they should be displayed in the 'Bar' normally to see pool occupation. const ACTIVE_SLOTS = ["running", "queued", "open"]; should have deferred ones.
If include_deferred is 'false', then this is correct.
For summary, it's the same I believe deferred slots for pool with 'include deferred=True' should be represented in the summary Bar. And bellow in the PoolBarCard, we should see 'scheduled' and remaining 'deferred' (without include_deferred activated).
Mainly the proplem here is that a Pool with include_deferred True will appear with 0 open slots, while the 'bar' could be empty of 'running' 'queued' and 'open' which is weird.
We could show the value of include_deffered in the tooltips to help people understand the difference between the two possible ways of displaying 'deferred' slots. (includ_deferred=True, include_deferred=False)
014ffe5 to
c606cca
Compare
|
Thank you @bbovenzi and @pierrejeambrun for the detailed review! I have updated the implementation based on your feedback:
I have verified the logic locally:
Ready for another look! |
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.
|
Thanks @pierrejeambrun for the detailed feedback! I have updated the implementation to fully address the UI styling and logic concerns. Changes included:
|
1366f60 to
2628e87
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.
I liked your original design idea, i.e if a state does not occupy a slot, we shot it under the bar. So we can have the full 'pull' state at a glance, and it's easy to understand what 'fills' the bar, and what is under the pool bar for information.
And then you don't need a big tooltip like that, we can keep the original tooltip, 1 per segment, and will highlight the 'state' in plain letters. Because here, when 'include_deferred=False', there is nothing shown for 'deferred slots' but the tooltip shows 'deferred' slots, and people will most likely get confused at to 'where those come from?'.
2628e87 to
160c37a
Compare
|
Hi @pierrejeambrun and @bbovenzi, I have squashed the commits and updated the logic based on your latest feedback:
Ready for final review! |
160c37a to
197e56e
Compare
197e56e to
4e7172d
Compare
|
Hi @bbovenzi updated based on your feedback:
|
4e7172d to
5ae9a7f
Compare
5ae9a7f to
6de8f86
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.
LGTM, thanks!
Just rebased to see if that solves the CI issue. (error seemed unrelated)
…ts (apache#59270) * UI: Update PoolBar to separate Scheduled and Deferred slots * UI: Use standard tooltips and cleanup i18n keys (cherry picked from commit 41f3dfe) Co-authored-by: Anshu Singh <anshu.singh@ksolves.com>
…9270) * UI: Update PoolBar to separate Scheduled and Deferred slots * UI: Use standard tooltips and cleanup i18n keys









Description
This PR addresses issue #53826.
Currently, the Pool Bar visually includes
ScheduledandDeferredstates inside the progress bar. This can be misleading as these states do not typically consume open pool slots, making the pool look "fuller" than it actually is.Changes introduced:
Running,Queued,Open) inside the visual bar.ScheduledandDeferredcounts as text/icons below the bar on the Pools administration page.Screenshots
Before:


After:



Related Issue
Closes #53826
^ 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.