Support @ServiceConnection endpoint attribute for S3Presigner
#1310
Conversation
| @@ -88,6 +93,14 @@ void configuresS3ClientWithServiceConnection(@Autowired S3Client client) { | |||
| assertThatCode(client::listBuckets).doesNotThrowAnyException(); | |||
| } | |||
|
|
|||
| @Test | |||
| @Disabled | |||
| void configuresS3PresignerWithServiceConnection(@Autowired S3Presigner s3Presigner) { | |||
There was a problem hiding this comment.
@maciejwalkowiak @MatejNedic
I have disabled this test because I am uncertain how to verify that the S3Presigner correctly references the endpoint property from the ServiceConnection. I noticed that most client tests simply call one of the basic methods and ensure that the call does not result in an exception.
In this case, I attempted to use the presignGetObject method for verification. However, this method requires several parameters to be set up, and it might also necessitate configuring an S3 bucket prior to running the test. This has left me uncertain whether my approach is correct. I would appreciate your opinion or guidance on how to proceed with this.
There was a problem hiding this comment.
@maciejwalkowiak @MatejNedic @tomazfernandes
I have fixed the test, please let me know if I have validated test correctly.
There was a problem hiding this comment.
@maciejwalkowiak @MatejNedic @tomazfernandes
Is there any way to retry the pipeline, because the test which is failing in the CI process is passing locally.
Reason: - Endpoint override conditions are added as else if blocks - Removed the Disable test annotation as added correct test for validating aws connection details with s3 presigner.
kunalvarpe
requested review from
maciejwalkowiak,
MatejNedic and
tomazfernandes
as code owners
Fixes: gh-1289
📢 Type of change
📜 Description
This change address the issue where endpoint property is not respected for S3Presigner bean auto configuration with
@ServiceConnection.💡 Motivation and Context
It fixes #1289 which reports the issue.
💚 How did you test it?
📝 Checklist
🔮 Next steps