SAGL-90: Invoke lambda using API #5296
Conversation
|
🎉 All dependencies have been resolved ! |
lib/lib-markdown/src/main/java/org/sagebionetworks/markdown/MarkdownDaoImpl.java
Show resolved
Hide resolved
lib/lib-markdown/src/main/java/org/sagebionetworks/markdown/MarkdownDaoImpl.java
Outdated
Show resolved
Hide resolved
lib/lib-markdown/src/main/java/org/sagebionetworks/markdown/MarkdownDaoImpl.java
Show resolved
Hide resolved
| ReflectionTestUtils.setField(dao, "markdownClient", mockMarkdownClient); | ||
| dao.setSynapseBaseUrl("https://www.synapse.org"); | ||
| dao.setStack("dev"); | ||
| ReflectionTestUtils.setField(dao, "lambdaClient", mockLambdaClient); |
There was a problem hiding this comment.
No need for the utility, use junit 5 @Injectmocks instead
lib/stackConfiguration/src/main/java/org/sagebionetworks/aws/AwsClientFactory.java
Outdated
Show resolved
Hide resolved
lib/lib-markdown/src/test/java/org/sagebionetworks/markdown/MarkdownDaoImplIntegrationTest.java
Show resolved
Hide resolved
| InvokeResult result = lambdaClient.invoke(invokeRequest); | ||
|
|
||
| if (result.getFunctionError() != null) { | ||
| throw new MarkdownClientException(500, "Lambda execution failed: " + result.getFunctionError()); |
There was a problem hiding this comment.
We should probably log more details from response (the docs says more details are in the result.getPayload(), also the result object has a getStatusCode() function that we can use instead of 500.
|
I also want to point out that I'm not sure why we need any of this in the first place. As far as I can tell from the code we only use it to convert some markdown into an html email, can't we just use an html template for the email as we do in other places? I do not see this used in the webclient and the current lambda URL is not publicly accessible (but I might be missing something) either so maybe we do not need the lambda at all? |
| import software.amazon.awssdk.services.lambda.LambdaClient; | ||
|
|
||
| @Configuration | ||
| @ImportResource("classpath:stack-configuration.spb.xml") |
| @RunWith(SpringJUnit4ClassRunner.class) | ||
| @ContextConfiguration(locations = {"classpath:test-context.xml"}) | ||
| @ExtendWith(SpringExtension.class) | ||
| @ContextConfiguration(classes = {MarkdownConfig.class}) |
| <!-- Load the single cloudwatch consumer --> | ||
| <import resource="classpath:cloudwatch-spb.xml" /> | ||
| <!-- Markdown service --> | ||
| <import resource="classpath:markdown-dao.spb.xml" /> |
There was a problem hiding this comment.
How is the MarkdownConfig imported where it is used? Maybe through the ManagerConfiguration? Unfortunately, the code structure is very convoluted and while this is only used by the workers application it is currently imported by the manager (which is also included in the repo API application).
| @Bean | ||
| @Scope("singleton") | ||
| public MarkdownDao markdownDao() { | ||
| MarkdownDaoImpl markdownDao = new MarkdownDaoImpl(lambdaClient(), stackConfiguration.getSynapseBaseUrl(), stackConfiguration.getStack()); |
There was a problem hiding this comment.
The StackConfiguration instance can be a parameter of this method rather than an object property, the LambdaClient does not need to be an exported bean actually, if it needs to be "shared" I would rather have it exposed as a bean somewhere else. In other words, I think it's best if the MarkdownConfig does not add to the context beans that are not related to this module.
2 participants
Depends on Sage-Bionetworks/synapse-markdown-it-lambda#3 to be deployed before building (need actual function name to call).