oscerd commented on PR #28625: URL: https://github.com/apache/camel/pull/28625#issuecomment-6477531405
Thanks for the review, @davsclaus. **1. The presigner is not closed when presigning fails.** Fixed in "CAMEL-26367: camel-aws2-s3 - close the presigner also when presigning fails", as you hinted. In `createDownloadLink` and `createUploadLink` the presigner is now created right before the presign call, and a try/finally around that call closes it when it was created locally (same `amazonS3Presigner` check as before). A failed `presignGetObject`/`presignPutObject` no longer leaks the presigner and its `DefaultCredentialsProvider`. A presigner set on the configuration is still never closed. - Closing right after the presign call is safe: the returned `PresignedGetObjectRequest`/`PresignedPutObjectRequest` is a value object. - The close cannot hide the presigning exception, because the SDK presigner closes its provider quietly (`IoUtils.closeIfCloseable`). `AWS2S3ProducerPresignerCredentialsTest` has three new tests: - `presignerIsClosedWhenPresigningTheDownloadLinkFails` and `presignerIsClosedWhenPresigningTheUploadLinkFails`: the presign call throws an `SdkClientException`, the same exception reaches the caller, and the presigner is closed. Both fail without the try/finally (`Wanted but not invoked: s3Presigner.close()`). - `presignerOfTheConfigurationIsNotClosedWhenPresigningFails`: a presigner from the configuration stays open on that path. **2. One presigner per producer (follow-up).** Thanks, good idea. When the PR was prepared, the minimal fix (one line per call site) was chosen on purpose, and caching the presigner per producer was left out of this regression fix. It can be a follow-up improvement if wanted. _Claude Code on behalf of oscerd_ -- 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]
