marta-jankovics commented on code in PR #3555:
URL: https://github.com/apache/fineract/pull/3555#discussion_r1386111802
##########
fineract-core/src/main/java/org/apache/fineract/batch/service/BatchApiServiceImpl.java:
##########
@@ -116,7 +116,7 @@ public List<BatchResponse>
handleBatchRequestsWithoutEnclosingTransaction(final
*/
@Override
public List<BatchResponse>
handleBatchRequestsWithEnclosingTransaction(final List<BatchRequest>
requestList, final UriInfo uriInfo) {
- return callInTransaction(Function.identity()::apply, () ->
handleBatchRequests(true, requestList, uriInfo));
+ return callInTransaction(Function.identity()::apply, () ->
handleBatchRequests(true, requestList, uriInfo), true);
Review Comment:
Could you please explain why this was needed? The enclosing transaction was
put both cases but
withEnclosingTransaction around the whole batch and
withoutEnclosingTransaction around each separate execution.
If you change this then there is a use-case with withoutEnclosingTransaction
which might not work: when the parent request fails then all the child requests
should also fail, in method parentRequestFailedRecursive we should mark the
child transactions to be rolled back. Is this working fine after your change?
Additionally this story is about to handle both with- and without enclosing
transaction cases - to save the failed command - and I do not see how this
helps in case of withEnclosing case.
With this change, as I see, the problem is not solved, but postponed.
##########
fineract-core/src/main/java/org/apache/fineract/commands/service/SynchronousCommandProcessingService.java:
##########
@@ -139,7 +139,8 @@ public CommandProcessingResult executeCommand(final
CommandWrapper wrapper, fina
commandSource.setCommandJson(toApiJsonSerializer.serializeResult(result.getChanges()));
}
- commandSource =
commandSourceService.saveResultSameTransaction(commandSource);
+ commandSource = sameTransaction ?
commandSourceService.saveResultSameTransaction(commandSource)
+ : commandSourceService.saveResultNewTransaction(commandSource);
Review Comment:
Can you please explain why you had "403 - Dublicate entry key" and how save
in new transaction helps?
As this is the success branch, it actually does not matter if we save in the
same or in new transaction, in both cases it will be saved.
Please make sure that you do not save new instance for each retry.
What is not happening right now is to save the command even if it was
failed, so the code that you should change is in the catch (Throwable t)
branch: instead of saveResultSameTransaction for 'sameTransaction' we should
call 'saveResultNewTransaction' each time. But what is missing though, is to
set the failed code at the end.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
