pierrejeambrun commented on code in PR #62831:
URL: https://github.com/apache/airflow/pull/62831#discussion_r2896544819
##########
airflow-core/src/airflow/ui/src/components/PoolBar.tsx:
##########
@@ -67,7 +71,14 @@ export const PoolBar = ({
{preparedSlots
.filter((slot) => barSlots.includes(slot.slotType) && slot.slotValue
> 0)
.map((slot) => {
- const flexValue = slot.slotValue / totalSlots || 0;
+ const usedSlots = preparedSlots
+ .filter((s) => barSlots.includes(s.slotType) && s.slotValue > 0
&& s.slotType !== "open")
+ .reduce((sum, s) => sum + s.slotValue, 0);
+ const flexValue = isUnlimited
+ ? slot.slotType === "open"
+ ? Math.max(1, usedSlots) // open takes at least as much space
as all used slots combined
+ : slot.slotValue
+ : slot.slotValue / totalSlots || 0;
Review Comment:
This piece is weird. Why don't we just re-use `slot`, which is already
`preparedSlots` but filtered. We just need to further filter on the `open`, but
starting again from `preparedSlots` and dupliacting the filters is confusing.
##########
airflow-core/src/airflow/ui/src/components/PoolBar.tsx:
##########
@@ -67,7 +71,14 @@ export const PoolBar = ({
{preparedSlots
.filter((slot) => barSlots.includes(slot.slotType) && slot.slotValue
> 0)
.map((slot) => {
- const flexValue = slot.slotValue / totalSlots || 0;
+ const usedSlots = preparedSlots
Review Comment:
This 'usedSlots' is a constant, it doesn't need to be re-computed in each
'.map` iteration and can probably be taken out of the loop.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
