davsclaus commented on code in PR #14238:
URL: https://github.com/apache/camel/pull/14238#discussion_r1614378502


##########
components/camel-aws/camel-aws2-kinesis/src/main/java/org/apache/camel/component/aws2/kinesis/Kinesis2Producer.java:
##########
@@ -47,11 +53,28 @@ public Kinesis2Endpoint getEndpoint() {
 
     @Override
     public void process(Exchange exchange) throws Exception {
+        Exception exceptionThrownByPutRecord = this.exceptionThrownByPutRecord;
+        if (exceptionThrownByPutRecord != null) {
+            this.exceptionThrownByPutRecord = null;
+            throw exceptionThrownByPutRecord;
+        }
+
         PutRecordRequest request = createRequest(exchange);
-        PutRecordResponse putRecordResult = 
connection.getClient(getEndpoint()).putRecord(request);
-        Message message = getMessageForResponse(exchange);
-        message.setHeader(Kinesis2Constants.SEQUENCE_NUMBER, 
putRecordResult.sequenceNumber());
-        message.setHeader(Kinesis2Constants.SHARD_ID, 
putRecordResult.shardId());
+        if (getEndpoint().getConfiguration().isAsyncClient()) {

Review Comment:
   This is wrong, as you cannot use async client in a camel producer that is 
fully synchronous. Camel needs to be aware that this is async and route 
accordingly - this kind of work is complex and takes time to understand how to 
do it.



##########
components/camel-aws/camel-aws2-kinesis/src/main/java/org/apache/camel/component/aws2/kinesis/Kinesis2Producer.java:
##########
@@ -47,11 +53,28 @@ public Kinesis2Endpoint getEndpoint() {
 
     @Override
     public void process(Exchange exchange) throws Exception {
+        Exception exceptionThrownByPutRecord = this.exceptionThrownByPutRecord;

Review Comment:
   This is not as good as concurrent calls could lead to thread safety issues - 
Use AtomicInstance or what the name is.
   
   Also its not understandable why a previous exception should cause the next 
to fail. There is no batch stuff here. 
   
   I think you should seperate the items you try to help with
   
   - batching (make this work with current client)
   - async (much harder - wait with this as its not needed asap)



-- 
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: commits-unsubscr...@camel.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to