Skip to content

Handle errors when getting modifications#61

Merged
FranckLecuyer merged 30 commits intomainfrom
handle_errors_when_getting_modifications
Mar 24, 2026
Merged

Handle errors when getting modifications#61
FranckLecuyer merged 30 commits intomainfrom
handle_errors_when_getting_modifications

Conversation

@FranckLecuyer
Copy link
Contributor

@FranckLecuyer FranckLecuyer commented Mar 4, 2026

PR Summary

Handle errors when getting modifications

based on PR : #58
to be merged together with PR : gridsuite/network-modification-server#775

Signed-off-by: Franck LECUYER <franck.lecuyer@rte-france.com>
…onfig

Signed-off-by: Franck LECUYER <franck.lecuyer@rte-france.com>

# Conflicts:
#	monitor-commons/src/main/java/org/gridsuite/monitor/commons/SecurityAnalysisConfig.java
#	monitor-server/src/main/java/org/gridsuite/monitor/server/entities/SecurityAnalysisConfigEntity.java
#	monitor-server/src/main/java/org/gridsuite/monitor/server/mapper/SecurityAnalysisConfigMapper.java
#	monitor-server/src/main/resources/db/changelog/db.changelog-master.yaml
#	monitor-server/src/test/java/org/gridsuite/monitor/server/MonitorIntegrationTest.java
#	monitor-server/src/test/java/org/gridsuite/monitor/server/controllers/ProcessConfigControllerTest.java
Signed-off-by: Franck LECUYER <franck.lecuyer@rte-france.com>
Signed-off-by: Franck LECUYER <franck.lecuyer@rte-france.com>
…config_uuid

Signed-off-by: Franck LECUYER <franck.lecuyer@rte-france.com>

# Conflicts:
#	monitor-worker-server/src/main/java/org/gridsuite/monitor/worker/server/services/SecurityAnalysisRestService.java
Signed-off-by: Franck LECUYER <franck.lecuyer@rte-france.com>
Signed-off-by: Franck LECUYER <franck.lecuyer@rte-france.com>
Signed-off-by: Franck LECUYER <franck.lecuyer@rte-france.com>
Signed-off-by: Franck LECUYER <franck.lecuyer@rte-france.com>
Don't set startedAt and completedAt for skipped step

Signed-off-by: Franck LECUYER <franck.lecuyer@rte-france.com>
Add process config id in the process execution entity

Signed-off-by: Franck LECUYER <franck.lecuyer@rte-france.com>
@coderabbitai

This comment was marked as outdated.

@FranckLecuyer FranckLecuyer changed the base branch from launch_process_using_config_uuid to main March 4, 2026 14:03
@FranckLecuyer FranckLecuyer changed the base branch from main to launch_process_using_config_uuid March 4, 2026 14:05
…config_uuid

Signed-off-by: Franck LECUYER <franck.lecuyer@rte-france.com>

# Conflicts:
#	monitor-worker-server/src/main/java/org/gridsuite/monitor/worker/server/services/StepExecutionService.java
#	monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/processes/securityanalysis/steps/SecurityAnalysisRunComputationStepTest.java
Signed-off-by: Franck LECUYER <franck.lecuyer@rte-france.com>
…' into handle_errors_when_getting_modifications

Signed-off-by: Franck LECUYER <franck.lecuyer@rte-france.com>

# Conflicts:
#	monitor-worker-server/src/main/java/org/gridsuite/monitor/worker/server/services/StepExecutionService.java
#	monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/processes/commons/steps/ApplyModificationsStepTest.java
Base automatically changed from launch_process_using_config_uuid to main March 12, 2026 08:55
…tting_modifications

Signed-off-by: Franck LECUYER <franck.lecuyer@rte-france.com>

# Conflicts:
#	monitor-commons/src/main/java/org/gridsuite/monitor/commons/SecurityAnalysisConfig.java
#	monitor-commons/src/main/java/org/gridsuite/monitor/commons/steps/AbstractStepExecutor.java
#	monitor-commons/src/test/java/org/gridsuite/monitor/commons/steps/StepExecutorTest.java
#	monitor-server/src/main/java/org/gridsuite/monitor/server/controllers/MonitorController.java
#	monitor-server/src/main/java/org/gridsuite/monitor/server/mapper/ProcessExecutionMapper.java
#	monitor-server/src/main/java/org/gridsuite/monitor/server/mapper/SecurityAnalysisConfigMapper.java
#	monitor-server/src/main/java/org/gridsuite/monitor/server/services/MonitorService.java
#	monitor-server/src/main/resources/db/changelog/changesets/changelog_20260227T101113Z.xml
#	monitor-server/src/test/java/org/gridsuite/monitor/server/MonitorIntegrationTest.java
#	monitor-server/src/test/java/org/gridsuite/monitor/server/controllers/MonitorControllerTest.java
#	monitor-server/src/test/java/org/gridsuite/monitor/server/services/MonitorServiceTest.java
#	monitor-server/src/test/java/org/gridsuite/monitor/server/services/ProcessConfigServiceTest.java
#	monitor-worker-server/pom.xml
#	monitor-worker-server/src/main/java/org/gridsuite/monitor/worker/server/processes/commons/steps/ApplyModificationsStep.java
#	monitor-worker-server/src/main/java/org/gridsuite/monitor/worker/server/processes/securityanalysis/steps/SecurityAnalysisRunComputationStep.java
#	monitor-worker-server/src/main/java/org/gridsuite/monitor/worker/server/services/ActionsRestService.java
#	monitor-worker-server/src/main/java/org/gridsuite/monitor/worker/server/services/SecurityAnalysisParametersService.java
#	monitor-worker-server/src/main/java/org/gridsuite/monitor/worker/server/services/SecurityAnalysisRestService.java
#	monitor-worker-server/src/main/java/org/gridsuite/monitor/worker/server/services/StepExecutionService.java
#	monitor-worker-server/src/main/resources/org/gridsuite/monitor/worker/server/reports.properties
#	monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/processes/commons/steps/ApplyModificationsStepTest.java
#	monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/processes/securityanalysis/steps/SecurityAnalysisRunComputationStepTest.java
#	monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/services/ActionsRestServiceTest.java
#	monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/services/SecurityAnalysisRestServiceTest.java
#	monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/services/StepExecutionServiceTest.java
Signed-off-by: Franck LECUYER <franck.lecuyer@rte-france.com>
coderabbitai[bot]

This comment was marked as outdated.

Signed-off-by: Franck LECUYER <franck.lecuyer@rte-france.com>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/processes/commons/steps/ApplyModificationsStepTest.java (1)

102-119: Consider asserting the specific exception type.

The mocks are now properly set up (addressing previous feedback). However, the production code throws PowsyblException (as seen in ApplyModificationsStep.java line 94), but the test asserts RuntimeException.class. While this works because PowsyblException extends RuntimeException, using the specific type provides better test precision and ensures other runtime exceptions aren't inadvertently caught.

♻️ Proposed fix for more precise assertion
-        assertThrows(RuntimeException.class, () -> applyModificationsStep.execute(stepContext));
+        assertThrows(PowsyblException.class, () -> applyModificationsStep.execute(stepContext));

Add the import:

import com.powsybl.commons.PowsyblException;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/processes/commons/steps/ApplyModificationsStepTest.java`
around lines 102 - 119, Change the test to assert the specific PowsyblException
instead of RuntimeException: in ApplyModificationsStepTest (method
executeApplyModificationsFailWhenModificationsAreMissing) replace
assertThrows(RuntimeException.class, ...) with
assertThrows(PowsyblException.class, ...) and add the import
com.powsybl.commons.PowsyblException; this aligns the test with the actual
exception thrown by ApplyModificationsStep (PowsyblException).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/processes/commons/steps/ApplyModificationsStepTest.java`:
- Around line 102-119: Change the test to assert the specific PowsyblException
instead of RuntimeException: in ApplyModificationsStepTest (method
executeApplyModificationsFailWhenModificationsAreMissing) replace
assertThrows(RuntimeException.class, ...) with
assertThrows(PowsyblException.class, ...) and add the import
com.powsybl.commons.PowsyblException; this aligns the test with the actual
exception thrown by ApplyModificationsStep (PowsyblException).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9611a4fb-9959-4409-8826-b085c9b2002e

📥 Commits

Reviewing files that changed from the base of the PR and between ddc3801 and 60a59a3.

📒 Files selected for processing (1)
  • monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/processes/commons/steps/ApplyModificationsStepTest.java

coderabbitai[bot]

This comment was marked as outdated.

Signed-off-by: Franck LECUYER <franck.lecuyer@rte-france.com>
@FranckLecuyer FranckLecuyer force-pushed the handle_errors_when_getting_modifications branch from f84ea7c to a5fdbb7 Compare March 12, 2026 13:04
coderabbitai[bot]

This comment was marked as outdated.

Signed-off-by: Franck LECUYER <franck.lecuyer@rte-france.com>
Copy link
Contributor

@antoinebhs antoinebhs left a comment

Choose a reason for hiding this comment

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

I'm not sure to understand why we need this complexity?
On main, we know the ID we want:
networkModificationRestService.getModifications(modificationIds);
and the ID we get List modificationInfos.getUuid()
Why can't we deduce the missing modifications ?

FranckLecuyer and others added 2 commits March 18, 2026 10:16
Signed-off-by: Franck LECUYER <franck.lecuyer@rte-france.com>
coderabbitai[bot]

This comment was marked as outdated.

FranckLecuyer and others added 3 commits March 19, 2026 10:35
…_modifications

# Conflicts:
#	monitor-worker-server/src/main/java/org/gridsuite/monitor/worker/server/process/commons/steps/ApplyModificationsStep.java
#	monitor-worker-server/src/main/java/org/gridsuite/monitor/worker/server/services/SecurityAnalysisParametersService.java
#	monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/clients/NetworkModificationRestClientTest.java
#	monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/orchestrator/StepExecutionServiceTest.java
#	monitor-worker-server/src/test/java/org/gridsuite/monitor/worker/server/process/commons/steps/ApplyModificationsStepTest.java
Copy link
Contributor

@antoinebhs antoinebhs left a comment

Choose a reason for hiding this comment

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

Some maybe to discuss? let me know

Signed-off-by: Franck LECUYER <franck.lecuyer@rte-france.com>
@sonarqubecloud
Copy link

Copy link
Contributor

@antoinebhs antoinebhs left a comment

Choose a reason for hiding this comment

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

Code OK 👍

@FranckLecuyer FranckLecuyer merged commit 56a4ba0 into main Mar 24, 2026
4 checks passed
@FranckLecuyer FranckLecuyer deleted the handle_errors_when_getting_modifications branch March 24, 2026 09:46
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