[
https://issues.apache.org/jira/browse/BEAM-5062?focusedWorklogId=136142&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-136142
]
ASF GitHub Bot logged work on BEAM-5062:
----------------------------------------
Author: ASF GitHub Bot
Created on: 20/Aug/18 14:38
Start Date: 20/Aug/18 14:38
Worklog Time Spent: 10m
Work Description: iemejia edited a comment on issue #6122: [BEAM-5062]
Add an option to enable path style access to S3 buckets
URL: https://github.com/apache/beam/pull/6122#issuecomment-414338805
Hi @yuppie-flu my total excuses for taking so long on the review of your PR.
Thanks for the contribution, the code is ok for what it does, and it is
definitely a nice contribution.
I am puzzled on how to solve the issue for the global case. I mean, if we
expose all the options of S3 in S3Options we will have an unmanageable (and
error-prone) API, I was wondering if we could better add a method like
`withS3ClientFactory` where can parametrize the building of `AmazonS3Client`.
That way we could cover not only the pathstyle case but any new change in the
API because the user could always override and build the client differently if
needed.
Would you be up to work on this solution better?, the general idea is to
replace the instantiation of `AmazonS3` in `S3FileSystem` by something that
invokes this from `S3Options` (with the idea of being able to override it):
```
class S3ClientFactory implements
DefaultValueFactory<AmazonS3ClientBuilder> {
@Override
public AmazonS3ClientBuilder create(PipelineOptions pipelineOptions) {
S3Options options = pipelineOptions.as(S3Options.class);
AmazonS3ClientBuilder builder =
AmazonS3ClientBuilder.standard().withCredentials(options.getAwsCredentialsProvider());
if (options.getClientConfiguration() != null) {
builder =
builder.withClientConfiguration(options.getClientConfiguration());
}
if (Strings.isNullOrEmpty(options.getAwsServiceEndpoint())) {
builder = builder.withRegion(options.getAwsRegion());
} else {
builder =
builder.withEndpointConfiguration(
new
AwsClientBuilder.EndpointConfiguration(options.getAwsServiceEndpoint(),
options.getAwsRegion()));
}
return builder;
}
}
```
Of course end user could just reuse this class by using their own like this:
```
options.withS3ClientFactory(new MyClientFactory());
```
and
```
class MyClientFactory implements DefaultValueFactory<AmazonS3> {
@Override
public AmazonS3 create(PipelineOptions options) {
AmazonS3ClientBuilder builder = new S3ClientFactory().create(options);
builder.enablePathStyleAccess();
return builder.build();
}
}
```
WDYT ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 136142)
Time Spent: 40m (was: 0.5h)
> Add S3Options to enable path style access
> -----------------------------------------
>
> Key: BEAM-5062
> URL: https://issues.apache.org/jira/browse/BEAM-5062
> Project: Beam
> Issue Type: Improvement
> Components: io-java-aws
> Affects Versions: 2.5.0
> Reporter: Kirill Kozlov
> Assignee: Ismaël Mejía
> Priority: Minor
> Time Spent: 40m
> Remaining Estimate: 0h
>
> There are some implementations of S3, that does not support
> virtual-hosted-style URLs for buckets, only path-style.
> It would be useful to add one more option into S3Options to enable path-style
> access for buckets.
> [withPathStyleAccessEnabled|https://docs.aws.amazon.com/AWSJavaSDK/latest/javadoc/com/amazonaws/services/s3/AmazonS3Builder.html#withPathStyleAccessEnabled-java.lang.Boolean-]
> method could be used for it when building client.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
