-
Notifications
You must be signed in to change notification settings - Fork 6
adjust getting computation status #4068
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,7 @@ import { getComputationRunningStatus, RunningStatus } from 'components/utils/run | |
| import type { UUID } from 'node:crypto'; | ||
| import { RefObject, useCallback, useEffect, useRef } from 'react'; | ||
| import { useDispatch } from 'react-redux'; | ||
| import { ComputingType, NotificationsUrlKeys, useNotificationsListener } from '@gridsuite/commons-ui'; | ||
| import { BuildStatus, ComputingType, NotificationsUrlKeys, useNotificationsListener } from '@gridsuite/commons-ui'; | ||
| import { setComputingStatus, setComputingStatusParameters, setLastCompletedComputation } from '../../redux/actions'; | ||
| import { AppDispatch } from '../../redux/store'; | ||
| import { parseEventData, StudyUpdatedEventData } from '../../types/notification-types'; | ||
|
|
@@ -20,6 +20,7 @@ interface UseComputingStatusProps { | |
| studyUuid: UUID, | ||
| nodeUuid: UUID, | ||
| currentRootNetworkUuid: UUID, | ||
| currentNodeBuildStatus: BuildStatus, | ||
| computingStatusFetcher: ( | ||
| studyUuid: UUID, | ||
| nodeUuid: UUID, | ||
|
|
@@ -37,11 +38,16 @@ function isWorthUpdate( | |
| nodeUuidRef: RefObject<UUID | null>, | ||
| rootNetworkUuidRef: RefObject<UUID | null>, | ||
| nodeUuid: UUID, | ||
| currentRootNetworkUuid: UUID | ||
| currentRootNetworkUuid: UUID, | ||
| notificationNode: UUID | undefined | ||
| ): boolean { | ||
| if (rootNetworkUuidRef.current !== currentRootNetworkUuid) { | ||
| return true; | ||
| } | ||
| // if notification is about a node that is not current node, no need to update current node computation status | ||
| if (notificationNode && notificationNode !== nodeUuid) { | ||
| return false; | ||
| } | ||
| if (nodeUuidRef.current !== nodeUuid) { | ||
| return true; | ||
| } | ||
|
|
@@ -67,6 +73,7 @@ const shouldRequestBeCanceled = ( | |
| * this hook loads all <computingType> state into redux at once, then updates it according to notifications for all computation | ||
| * @param studyUuid current study uuid | ||
| * @param nodeUuid current node uuid | ||
| * @param currentNodeBuildStatus | ||
| * @param allComputingStatusFetcher method fetching all <computingType> state | ||
| * @param currentRootNetworkUuid | ||
| * @param computingStatusParametersFetcherMap | ||
|
|
@@ -75,6 +82,7 @@ export const useAllComputingStatusAtOnce: UseComputingStatusProps = ( | |
| studyUuid, | ||
| nodeUuid, | ||
| currentRootNetworkUuid, | ||
| currentNodeBuildStatus, | ||
| allComputingStatusFetcher, | ||
| computingStatusParametersFetcherMap | ||
| ) => { | ||
|
|
@@ -202,18 +210,28 @@ export const useAllComputingStatusAtOnce: UseComputingStatusProps = ( | |
| const eventData = parseEventData<StudyUpdatedEventData>(event ?? null); | ||
| const headers = eventData?.headers; | ||
| const updateType = headers?.updateType; | ||
| const notificationNode = headers?.node; | ||
| // no need to request the back if node is not built | ||
| if (currentNodeBuildStatus === BuildStatus.NOT_BUILT) { | ||
| Object.values(ComputingType).forEach((computingType: ComputingType) => { | ||
| dispatch(setComputingStatus(computingType, RunningStatus.IDLE)); | ||
| }); | ||
| nodeUuidRef.current = nodeUuid; | ||
| return; | ||
|
Comment on lines
+214
to
+220
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win Invalidate outstanding updates before returning for This branch forces 🤖 Prompt for AI Agents |
||
| } | ||
| const isUpdateForUs = isWorthUpdate( | ||
| updateType, | ||
| nodeUuidRef, | ||
| rootNetworkUuidRef, | ||
| nodeUuid, | ||
| currentRootNetworkUuid | ||
| currentRootNetworkUuid, | ||
| notificationNode | ||
| ); | ||
| if (isUpdateForUs) { | ||
| updateAll(updateType); | ||
| } | ||
| }, | ||
| [currentRootNetworkUuid, nodeUuid, studyUuid, updateAll] | ||
| [currentNodeBuildStatus, currentRootNetworkUuid, dispatch, nodeUuid, studyUuid, updateAll] | ||
| ); | ||
|
|
||
| // evaluate at each notification | ||
|
|
||
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.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: gridsuite/gridstudy-app
Length of output: 13338
🏁 Script executed:
Repository: gridsuite/gridstudy-app
Length of output: 18556
Thread
currentNodeStatusBuildStatusinto the per-type hooks too —useAllComputingStatusAtOnce(...)is the only place that sees theBuildStatus, butuseComputingStatus(...)is still mounted for every computation type and still fetches/listens on its own. IfNOT_BUILTshould stayIDLE, those hooks can repopulate status after the batch reset.🤖 Prompt for AI Agents