bbovenzi commented on code in PR #45587: URL: https://github.com/apache/airflow/pull/45587#discussion_r1915565759
########## airflow/ui/src/pages/Pools/PoolBar.tsx: ########## @@ -0,0 +1,109 @@ +/*! + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { Box, Flex, Text, VStack } from "@chakra-ui/react"; +import { MdHourglassDisabled, MdHourglassFull } from "react-icons/md"; + +import type { PoolResponse } from "openapi/requests/types.gen"; +import { Tooltip } from "src/components/ui"; +import { stateColor } from "src/utils/stateColor"; + +type PoolBarProps = { + readonly pools: Array<PoolResponse>; +}; + +const PoolBar = ({ pools }: PoolBarProps) => ( + <Flex direction="column" gap={4}> + {pools.map((pool) => { + // Calculate proportions + const totalSlots = pool.slots; + const openFlex = pool.open_slots / totalSlots || 0; + const scheduledFlex = pool.scheduled_slots / totalSlots || 0; + const runningFlex = pool.running_slots / totalSlots || 0; + const queuedFlex = pool.queued_slots / totalSlots || 0; + const occupiedFlex = pool.occupied_slots / totalSlots || 0; + const deferredFlex = (pool.include_deferred ? pool.deferred_slots / totalSlots : 0) || 0; + + return ( + <Box + borderColor="border.emphasized" + borderRadius={8} + borderWidth={1} + key={pool.name} + overflow="hidden" + > + <Flex alignItems="center" bg="bg.muted" justifyContent="space-between" p={4}> + <VStack align="start"> + <Text fontSize="lg" fontWeight="bold"> + {pool.name} + </Text> + {Boolean(pool.description) ? ( + <Text color="gray.fg" fontSize="sm"> + {pool.description} + </Text> + ) : undefined} + </VStack> + <Tooltip + content={pool.include_deferred ? "Deferred Slots Included" : "Deferred Slots Not Included"} + > + {pool.include_deferred ? <MdHourglassFull size={25} /> : <MdHourglassDisabled size={25} />} + </Tooltip> + </Flex> + + <Box margin={4}> + <Flex bg="gray.100" borderRadius="md" h="20px" overflow="hidden" w="100%"> + {/* Open Slots */} + <Tooltip content={`Open Slots: ${pool.open_slots}`}> + <Box bg={stateColor.success} flex={openFlex} h="100%" /> + </Tooltip> + + {/* Scheduled Slots */} + <Tooltip content={`Scheduled Slots: ${pool.scheduled_slots}`}> + <Box bg={stateColor.scheduled} flex={scheduledFlex} h="100%" /> + </Tooltip> + + {/* Running Slots */} + <Tooltip content={`Running Slots: ${pool.running_slots}`}> + <Box bg={stateColor.running} flex={runningFlex} h="100%" /> + </Tooltip> + + {/* Queued Slots */} + <Tooltip content={`Queued Slots: ${pool.queued_slots}`}> + <Box bg={stateColor.queued} flex={queuedFlex} h="100%" /> + </Tooltip> + + {/* Occupied Slots */} + <Tooltip content={`Occupied Slots: ${pool.occupied_slots}`}> + <Box bg={stateColor.up_for_retry} flex={occupiedFlex} h="100%" /> + </Tooltip> + + {/* Deferred Slots */} + {pool.include_deferred && pool.deferred_slots > 0 ? ( Review Comment: ```suggestion {pool.include_deferred ? ( ``` If it's 0 then nothing really renders anyway. ########## airflow/ui/src/pages/Pools/PoolBar.tsx: ########## @@ -0,0 +1,109 @@ +/*! + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { Box, Flex, Text, VStack } from "@chakra-ui/react"; +import { MdHourglassDisabled, MdHourglassFull } from "react-icons/md"; + +import type { PoolResponse } from "openapi/requests/types.gen"; +import { Tooltip } from "src/components/ui"; +import { stateColor } from "src/utils/stateColor"; + +type PoolBarProps = { + readonly pools: Array<PoolResponse>; +}; + +const PoolBar = ({ pools }: PoolBarProps) => ( + <Flex direction="column" gap={4}> + {pools.map((pool) => { + // Calculate proportions + const totalSlots = pool.slots; + const openFlex = pool.open_slots / totalSlots || 0; + const scheduledFlex = pool.scheduled_slots / totalSlots || 0; + const runningFlex = pool.running_slots / totalSlots || 0; + const queuedFlex = pool.queued_slots / totalSlots || 0; + const occupiedFlex = pool.occupied_slots / totalSlots || 0; + const deferredFlex = (pool.include_deferred ? pool.deferred_slots / totalSlots : 0) || 0; + + return ( + <Box + borderColor="border.emphasized" + borderRadius={8} + borderWidth={1} + key={pool.name} + overflow="hidden" + > + <Flex alignItems="center" bg="bg.muted" justifyContent="space-between" p={4}> + <VStack align="start"> + <Text fontSize="lg" fontWeight="bold"> + {pool.name} + </Text> + {Boolean(pool.description) ? ( + <Text color="gray.fg" fontSize="sm"> + {pool.description} + </Text> + ) : undefined} + </VStack> + <Tooltip + content={pool.include_deferred ? "Deferred Slots Included" : "Deferred Slots Not Included"} + > + {pool.include_deferred ? <MdHourglassFull size={25} /> : <MdHourglassDisabled size={25} />} + </Tooltip> + </Flex> + + <Box margin={4}> + <Flex bg="gray.100" borderRadius="md" h="20px" overflow="hidden" w="100%"> Review Comment: ```suggestion <Flex bg="bg.muted" borderRadius="md" h="20px" overflow="hidden" w="100%"> ``` let's try to use the semantic color tokens. ########## airflow/ui/src/pages/Pools/PoolBar.tsx: ########## @@ -0,0 +1,109 @@ +/*! + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { Box, Flex, Text, VStack } from "@chakra-ui/react"; +import { MdHourglassDisabled, MdHourglassFull } from "react-icons/md"; + +import type { PoolResponse } from "openapi/requests/types.gen"; +import { Tooltip } from "src/components/ui"; +import { stateColor } from "src/utils/stateColor"; + +type PoolBarProps = { + readonly pools: Array<PoolResponse>; +}; + +const PoolBar = ({ pools }: PoolBarProps) => ( + <Flex direction="column" gap={4}> + {pools.map((pool) => { Review Comment: I think this part should be in `Pools.tsx` and this component should be for only a single Pool. ########## airflow/ui/src/pages/Pools/PoolBar.tsx: ########## @@ -0,0 +1,109 @@ +/*! + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { Box, Flex, Text, VStack } from "@chakra-ui/react"; +import { MdHourglassDisabled, MdHourglassFull } from "react-icons/md"; + +import type { PoolResponse } from "openapi/requests/types.gen"; +import { Tooltip } from "src/components/ui"; +import { stateColor } from "src/utils/stateColor"; + +type PoolBarProps = { + readonly pools: Array<PoolResponse>; +}; + +const PoolBar = ({ pools }: PoolBarProps) => ( + <Flex direction="column" gap={4}> + {pools.map((pool) => { + // Calculate proportions + const totalSlots = pool.slots; + const openFlex = pool.open_slots / totalSlots || 0; + const scheduledFlex = pool.scheduled_slots / totalSlots || 0; + const runningFlex = pool.running_slots / totalSlots || 0; + const queuedFlex = pool.queued_slots / totalSlots || 0; + const occupiedFlex = pool.occupied_slots / totalSlots || 0; + const deferredFlex = (pool.include_deferred ? pool.deferred_slots / totalSlots : 0) || 0; Review Comment: What if we declared a const of slot types at the top of this files. It can be a dict of keys and the colors for each. Then we can map over it inside the component here and below so all this code can be more DRY. ex: ``` const slots = { 'open_slots': stateColor.success, 'scheduled_slots': stateColor.scheduled, . . } ``` ########## airflow/ui/src/pages/Pools/PoolBar.tsx: ########## @@ -0,0 +1,109 @@ +/*! + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { Box, Flex, Text, VStack } from "@chakra-ui/react"; +import { MdHourglassDisabled, MdHourglassFull } from "react-icons/md"; + +import type { PoolResponse } from "openapi/requests/types.gen"; +import { Tooltip } from "src/components/ui"; +import { stateColor } from "src/utils/stateColor"; + +type PoolBarProps = { + readonly pools: Array<PoolResponse>; +}; + +const PoolBar = ({ pools }: PoolBarProps) => ( + <Flex direction="column" gap={4}> + {pools.map((pool) => { + // Calculate proportions + const totalSlots = pool.slots; + const openFlex = pool.open_slots / totalSlots || 0; + const scheduledFlex = pool.scheduled_slots / totalSlots || 0; + const runningFlex = pool.running_slots / totalSlots || 0; + const queuedFlex = pool.queued_slots / totalSlots || 0; + const occupiedFlex = pool.occupied_slots / totalSlots || 0; + const deferredFlex = (pool.include_deferred ? pool.deferred_slots / totalSlots : 0) || 0; + + return ( + <Box + borderColor="border.emphasized" + borderRadius={8} + borderWidth={1} + key={pool.name} + overflow="hidden" + > + <Flex alignItems="center" bg="bg.muted" justifyContent="space-between" p={4}> + <VStack align="start"> + <Text fontSize="lg" fontWeight="bold"> + {pool.name} + </Text> + {Boolean(pool.description) ? ( + <Text color="gray.fg" fontSize="sm"> + {pool.description} + </Text> + ) : undefined} Review Comment: Let's include the total number of slots with the name and description ########## airflow/ui/src/pages/Pools/PoolBar.tsx: ########## @@ -0,0 +1,109 @@ +/*! + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { Box, Flex, Text, VStack } from "@chakra-ui/react"; +import { MdHourglassDisabled, MdHourglassFull } from "react-icons/md"; + +import type { PoolResponse } from "openapi/requests/types.gen"; +import { Tooltip } from "src/components/ui"; +import { stateColor } from "src/utils/stateColor"; + +type PoolBarProps = { + readonly pools: Array<PoolResponse>; +}; + +const PoolBar = ({ pools }: PoolBarProps) => ( + <Flex direction="column" gap={4}> + {pools.map((pool) => { + // Calculate proportions + const totalSlots = pool.slots; + const openFlex = pool.open_slots / totalSlots || 0; + const scheduledFlex = pool.scheduled_slots / totalSlots || 0; + const runningFlex = pool.running_slots / totalSlots || 0; + const queuedFlex = pool.queued_slots / totalSlots || 0; + const occupiedFlex = pool.occupied_slots / totalSlots || 0; + const deferredFlex = (pool.include_deferred ? pool.deferred_slots / totalSlots : 0) || 0; + + return ( + <Box + borderColor="border.emphasized" + borderRadius={8} + borderWidth={1} + key={pool.name} + overflow="hidden" + > + <Flex alignItems="center" bg="bg.muted" justifyContent="space-between" p={4}> + <VStack align="start"> + <Text fontSize="lg" fontWeight="bold"> + {pool.name} + </Text> + {Boolean(pool.description) ? ( + <Text color="gray.fg" fontSize="sm"> + {pool.description} + </Text> + ) : undefined} + </VStack> + <Tooltip + content={pool.include_deferred ? "Deferred Slots Included" : "Deferred Slots Not Included"} + > + {pool.include_deferred ? <MdHourglassFull size={25} /> : <MdHourglassDisabled size={25} />} Review Comment: ```suggestion {pool.include_deferred ? <MdHourglassFull size={25} /> : <MdHourglassDisabled size={25} />} ``` This is way too big. Let's do a max of 18 -- 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]
