Skip to content

Robustify node building / unbuilding#976

Open
basseche wants to merge 5 commits into
mainfrom
robustify_node_build
Open

Robustify node building / unbuilding#976
basseche wants to merge 5 commits into
mainfrom
robustify_node_build

Conversation

@basseche

@basseche basseche commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

PR Summary

Robustify node building :

@basseche basseche self-assigned this Apr 9, 2026
@coderabbitai

coderabbitai Bot commented Apr 9, 2026

Copy link
Copy Markdown

Caution

Review failed

An error occurred during the review process. Please try again later.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/main/java/org/gridsuite/study/server/controller/StudyController.java`:
- Line 1678: The current call
rootNetworkNodeInfoService.assertNoBuildingNode(rootNetworkUuid,
List.of(nodeUuid)) only checks the single target node; change it to collect all
UUIDs for the whole branch/path that could be affected (e.g. via the service
that resolves branch nodes — for example getBranchNodeUuids(rootNetworkUuid,
nodeUuid) or traverse parents/children) and pass that full list into
rootNetworkNodeInfoService.assertNoBuildingNode; update the code around the call
in StudyController (method handling the unbuild operation) to compute
relatedBranchUuids and call assertNoBuildingNode(rootNetworkUuid,
relatedBranchUuids) so concurrent BUILDING states on any branch node are
guarded.

In `@src/main/java/org/gridsuite/study/server/service/ConsumerService.java`:
- Around line 133-135: handleBuildSuccess(...) is currently invoked before
handleBuildResultWorkflow(...) and only JsonProcessingException is caught, so
runtime errors from workflow metadata parsing (e.g., WorkflowType.valueOf) can
crash the consumer after the build side-effects have been applied; wrap the call
to handleBuildResultWorkflow(studyUuid, receiverObj.getNodeUuid(),
receiverObj.getRootNetworkUuid(), message) in its own try/catch (catch broad
RuntimeException or IllegalArgumentException/IllegalStateException as
appropriate) so parsing or workflow-specific errors are logged/handled without
propagating and without undoing the successful path performed by
studyService.handleBuildSuccess(...).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fcb67c53-9d5a-4f4d-9761-276dd18b55c9

📥 Commits

Reviewing files that changed from the base of the PR and between bbbfed2 and 08349c2.

📒 Files selected for processing (2)
  • src/main/java/org/gridsuite/study/server/controller/StudyController.java
  • src/main/java/org/gridsuite/study/server/service/ConsumerService.java

Comment thread src/main/java/org/gridsuite/study/server/controller/StudyController.java Outdated
Signed-off-by: basseche <bassel.el-cheikh_externe@rte-france.com>
@sonarqubecloud

Copy link
Copy Markdown

@basseche basseche closed this Jul 9, 2026
@SlimaneAmar SlimaneAmar reopened this Jul 16, 2026
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants