Skip to content
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

feat: rely on rabbitmq dlq for computation errors handling #156

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@
<dependencyManagement>
<dependencies>
<!-- overrides of imports -->
<dependency>
Copy link
Contributor

Choose a reason for hiding this comment

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

to remove next powsybl upgrade?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will update with the released version of powsybl-ws-commons anyway, so I will ad the comment

<groupId>com.powsybl</groupId>
<artifactId>powsybl-ws-commons</artifactId>
<version>1.19.0-SNAPSHOT</version>
</dependency>
<dependency><!-- To remove when integrate in next release of gridsuite-dependencies or powsybl-ws-dependencies -->
<groupId>com.squareup.okhttp3</groupId>
<artifactId>okhttp-bom</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,6 @@ public SecurityAnalysisResult run(SecurityAnalysisRunContext runContext) {
runContext.setNetwork(network);
AtomicReference<ReportNode> rootReporter = new AtomicReference<>();
return run(runContext, null, rootReporter);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
return null;
} catch (Exception e) {
LOGGER.error(getFailedMessage(getComputationType()), e);
return null;
Expand Down
21 changes: 16 additions & 5 deletions src/main/resources/config/application.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
spring:
application:
name: security-analysis-server

jpa:
properties:
hibernate:
Expand All @@ -20,6 +20,7 @@ spring:
group: saGroup
consumer:
concurrency: 2
max-attempts: 1
publishRun-out-0:
destination: ${powsybl-ws.rabbitmq.destination.prefix:}sa.run
publishResult-out-0:
Expand All @@ -30,11 +31,21 @@ spring:
destination: ${powsybl-ws.rabbitmq.destination.prefix:}sa.cancel
publishStopped-out-0:
destination: ${powsybl-ws.rabbitmq.destination.prefix:}sa.stopped
publishFailed-out-0:
destination: ${powsybl-ws.rabbitmq.destination.prefix:}sa.failed
publishCancelFailed-out-0:
destination: ${powsybl-ws.rabbitmq.destination.prefix:}sa.cancelfailed
output-bindings: publishRun-out-0;publishResult-out-0;publishCancel-out-0;publishStopped-out-0;publishFailed-out-;publishCancelFailed-out-0
output-bindings: publishRun-out-0;publishResult-out-0;publishCancel-out-0;publishStopped-out-0;publishCancelFailed-out-0
rabbit:
bindings:
consumeRun-in-0:
consumer:
auto-bind-dlq: true
dead-letter-exchange: ${powsybl-ws.rabbitmq.destination.prefix:}sa.run.dlx
dead-letter-queue-name: ${powsybl-ws.rabbitmq.destination.prefix:}sa.run.dlx.dlq
dead-letter-exchange-type: topic
quorum:
enabled: true
delivery-limit: 2


powsybl-ws:
database:
Expand Down Expand Up @@ -89,4 +100,4 @@ security-analysis:
- [ 1, 1, 1, 1 ]
- [ 1, 1, 1, 1 ]
- [ 1, 1, 1, 1 ]
- [ 1, 1, 1, 1 ]
- [ 1, 1, 1, 1 ]
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
import static org.gridsuite.securityanalysis.server.util.TestUtils.assertLogMessage;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.doNothing;
Expand Down Expand Up @@ -673,11 +674,8 @@ void runTestWithError() throws Exception {
UUID resultUuid = mapper.readValue(resultAsString, UUID.class);
assertEquals(RESULT_UUID, resultUuid);

// Message stopped has been sent
Message<byte[]> cancelMessage = output.receive(TIMEOUT, "sa.failed");
assertEquals(RESULT_UUID.toString(), cancelMessage.getHeaders().get("resultUuid"));
assertEquals("me", cancelMessage.getHeaders().get("receiver"));
assertEquals(getFailedMessage(COMPUTATION_TYPE) + " : " + ERROR_MESSAGE, cancelMessage.getHeaders().get("message"));
// No result message
assertNull(output.receive(TIMEOUT, "sa.result"));
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the equivalent test is to check if there is a .ComputationException with the corresponding message?

Copy link
Contributor

Choose a reason for hiding this comment

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

can we check the run.dlx?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no sadly it doesn't work... the default test binder setup by springcloudstream doesn't behave the same, then I couldn't test like that


// No result
assertResultNotFound(RESULT_UUID);
Expand Down
Loading