Copilot commented on code in PR #9218:
URL: https://github.com/apache/seatunnel/pull/9218#discussion_r2055721672
##########
seatunnel-translation/seatunnel-translation-spark/seatunnel-translation-spark-3.3/src/main/java/org/apache/seatunnel/translation/spark/sink/SeaTunnelBatchWrite.java:
##########
@@ -89,7 +89,13 @@ public DataWriterFactory
createBatchWriterFactory(PhysicalWriteInfo info) {
public void commit(WriterCommitMessage[] messages) {
if (aggregatedCommitter != null) {
try {
- aggregatedCommitter.commit(combineCommitMessage(messages));
+ List<AggregatedCommitInfoT> aggregatedCommitInfoTS =
+
aggregatedCommitter.commit(combineCommitMessage(messages));
+ if (!aggregatedCommitInfoTS.isEmpty()) {
+ throw new RuntimeException(
+ "Aggregated commit info should be empty, but got
size: "
+ + aggregatedCommitInfoTS.size());
+ }
Review Comment:
Throwing a generic RuntimeException when aggregated commit info is non-empty
may obscure the root cause. Consider using a more specific exception type or
adding additional logging to capture the context for debugging.
```suggestion
String errorMessage = String.format(
"Aggregated commit info should be empty, but got
size: %d. Contents: %s",
aggregatedCommitInfoTS.size(),
aggregatedCommitInfoTS);
LOG.error(errorMessage);
throw new IllegalStateException(errorMessage);
}
```
--
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]
