Fix S3 Endpoint or Region configuration. #55
Conversation
Antiarchitect
force-pushed
the
fix-s3-endpoint-configuration
branch
from
5903dd3 to
b298db4
Compare
|
Cool, thanks a lot for handing in a fix! It looks like you used the wrong email which has no ECA attached |
Antiarchitect
force-pushed
the
fix-s3-endpoint-configuration
branch
from
b298db4 to
5ab68fd
Compare
Signed-off-by: Andrey Voronkov <avoronkov@enapter.com>
Antiarchitect
force-pushed
the
fix-s3-endpoint-configuration
branch
from
5ab68fd to
bba8bbf
Compare
|
Now it looks much better. If we have an endpoint we don't need a region at all. But let me test it a bit |
Signed-off-by: Andrey Voronkov <avoronkov@enapter.com>
|
Good news, everyone! |
| @@ -83,11 +83,10 @@ public ClientConfiguration awsClientConfiguration() { | |||
| public AmazonS3 amazonS3() { | |||
| final AmazonS3ClientBuilder s3ClientBuilder = AmazonS3ClientBuilder.standard() | |||
| .withCredentials(awsCredentialsProvider()).withClientConfiguration(awsClientConfiguration()); | |||
| if (!StringUtils.isEmpty(region)) { | |||
| if (!StringUtils.isEmpty(endpoint)) { | |||
| s3ClientBuilder.withEndpointConfiguration(new EndpointConfiguration(endpoint, "")); | |||
There was a problem hiding this comment.
Shouldn't you set region as well if it is provided via property aws.region?
There was a problem hiding this comment.
It is not necessary as it goes
There was a problem hiding this comment.
Sorry, for my briefness. I was thinking more about other users/programmers who maybe want to use manually set S3 endpoint and use AWS's SigV4 signing (which may require the region to be set). What do you think? Will this work for MinIO as well?
if (!StringUtils.isEmpty(endpoint)) {
final String signingRegion = StringUtils.isEmpty(region) ? "" : region;
s3ClientBuilder.withEndpointConfiguration(new EndpointConfiguration(endpoint, signingRegion));
} else if (!StringUtils.isEmpty(region)) {
s3ClientBuilder.withRegion(region);
}There was a problem hiding this comment.
Probably. I will try to build it and test tomorrow.
Indeed! Thanks a lot for providing a small "how to setup" for MinIO right away. I only have one minor remark about the region (see above) before we merge this PR to master |
Signed-off-by: Andrey Voronkov <avoronkov@enapter.com>
Antiarchitect
force-pushed
the
fix-s3-endpoint-configuration
branch
from
3dbcf87 to
2641942
Compare
|
@schabdo Sorry for the delay! I've just tested your variant with MinIO (aws.region + aws.s3.endpoint) and true AWS S3 (aws.region only) and all works fine! |
|
No worries, as you see I need sometimes a bit more time to answer, too 😄 |
No description provided.