Croway commented on PR #29274:
URL: https://github.com/apache/camel/pull/29274#issuecomment-6669786051

   @davsclaus thanks for the review. Point by point:
   
   **1. Aggregator with `completionFromBatchConsumer`**
   Agreed, any skipped object shifts the groups. I reproduced it with the S3 
fake, using a folder object and `includeFolders=false`: the groups were `[a, b, 
d] [e, f, h, i]`.
   - **Docs (d25859baffe8):** the NOTE and the upgrade-guide bullet now name 
`completionFromBatchConsumer`. The sentence above the NOTE now points to it 
(c6fbbda561c4).
   - **Workaround:** use `completionTimeout` *instead of* 
`completionFromBatchConsumer`, shorter than the consumer `delay`. That gave one 
group per poll: `[a, b] [d] [e, f] [h, i, j]`.
   - **Not as a safety net next to it:** I tried that. The aggregator's batch 
counter is not reset when a group completes by timeout, so the group of a later 
poll was split (`[h, i] [j]`).
   - **`total - skippedSoFar`:** I tried a variant of it, getting one object 
ahead so that the last exchange of a poll had the exact size. It does not help 
the groups:
     - the list strategies return the first exchange of the group, and the 
aggregator reads `CamelBatchSize` from that one;
     - with `includeBody=false` and a route slower than 60 s, the next object's 
stream is aborted again, which is what this PR fixes.
   
   **2. Hooks with `ignoreBody=true`**
   Confirmed: the HEAD path calls neither 
`includeS3Object(ResponseInputStream)` nor `createExchange(ResponseInputStream, 
String)`. The upgrade guide now says so (c6fbbda561c4). gnodet-bot raised the 
same point.
   
   **3. SSE-KMS**
   Confirmed against the AWS docs and the SDK 2.57.7 service model:
   - GetObject of an SSE-KMS object needs `kms:Decrypt`.
   - HeadObject needs only `s3:GetObject`. `kms:Decrypt` is needed only with 
`x-amz-checksum-mode`, and the SDK does not set that on HeadObject, which has 
no `httpChecksum` trait.
   
   So such an object is now consumed with `ignoreBody=true`, and deleted with 
`deleteAfterRead`. I added it to the Glacier bullet (c6fbbda561c4).
   
   **4. `LAST_MODIFIED` in the producer**
   gnodet asked for it, so it went into this PR for both paths: GET and HEAD 
now set `CamelAwsS3LastModified` and `CamelMessageTimestamp` (313534831697).
   
   _Claude Code on behalf of Federico Mariani_
   


-- 
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]

Reply via email to