mosche commented on a change in pull request #16760: URL: https://github.com/apache/beam/pull/16760#discussion_r808046772
########## File path: sdks/java/io/amazon-web-services2/src/main/java/org/apache/beam/sdk/io/aws2/options/AwsOptions.java ########## @@ -17,76 +17,118 @@ */ package org.apache.beam.sdk.io.aws2.options; +import java.net.URI; import org.apache.beam.sdk.annotations.Experimental; import org.apache.beam.sdk.annotations.Experimental.Kind; +import org.apache.beam.sdk.io.aws2.common.ClientBuilderFactory; +import org.apache.beam.sdk.io.aws2.common.HttpClientConfiguration; import org.apache.beam.sdk.options.Default; import org.apache.beam.sdk.options.DefaultValueFactory; import org.apache.beam.sdk.options.Description; import org.apache.beam.sdk.options.PipelineOptions; import org.checkerframework.checker.nullness.qual.Nullable; import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider; +import software.amazon.awssdk.auth.credentials.ContainerCredentialsProvider; import software.amazon.awssdk.auth.credentials.DefaultCredentialsProvider; +import software.amazon.awssdk.auth.credentials.EnvironmentVariableCredentialsProvider; +import software.amazon.awssdk.auth.credentials.ProfileCredentialsProvider; +import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider; +import software.amazon.awssdk.auth.credentials.SystemPropertyCredentialsProvider; import software.amazon.awssdk.core.exception.SdkClientException; import software.amazon.awssdk.http.apache.ProxyConfiguration; +import software.amazon.awssdk.regions.Region; import software.amazon.awssdk.regions.providers.DefaultAwsRegionProviderChain; -import software.amazon.awssdk.utils.AttributeMap; +import software.amazon.awssdk.services.sts.auth.StsAssumeRoleCredentialsProvider; /** * Options used to configure Amazon Web Services specific options such as credentials and region. */ @Experimental(Kind.SOURCE_SINK) public interface AwsOptions extends PipelineOptions { - /** AWS region used by the AWS client. */ - @Description("AWS region used by the AWS client") + /** Region used to configure AWS service clients. */ + @Description("Region used by AWS service clients") @Default.InstanceFactory(AwsRegionFactory.class) - String getAwsRegion(); + Region getAwsRegion(); Review comment: This is a breaking change to improve type safety. Seems low risk as setting options is typically done via args ########## File path: sdks/java/io/amazon-web-services2/src/main/java/org/apache/beam/sdk/io/aws2/options/AwsOptions.java ########## @@ -17,76 +17,118 @@ */ package org.apache.beam.sdk.io.aws2.options; +import java.net.URI; import org.apache.beam.sdk.annotations.Experimental; import org.apache.beam.sdk.annotations.Experimental.Kind; +import org.apache.beam.sdk.io.aws2.common.ClientBuilderFactory; +import org.apache.beam.sdk.io.aws2.common.HttpClientConfiguration; import org.apache.beam.sdk.options.Default; import org.apache.beam.sdk.options.DefaultValueFactory; import org.apache.beam.sdk.options.Description; import org.apache.beam.sdk.options.PipelineOptions; import org.checkerframework.checker.nullness.qual.Nullable; import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider; +import software.amazon.awssdk.auth.credentials.ContainerCredentialsProvider; import software.amazon.awssdk.auth.credentials.DefaultCredentialsProvider; +import software.amazon.awssdk.auth.credentials.EnvironmentVariableCredentialsProvider; +import software.amazon.awssdk.auth.credentials.ProfileCredentialsProvider; +import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider; +import software.amazon.awssdk.auth.credentials.SystemPropertyCredentialsProvider; import software.amazon.awssdk.core.exception.SdkClientException; import software.amazon.awssdk.http.apache.ProxyConfiguration; +import software.amazon.awssdk.regions.Region; import software.amazon.awssdk.regions.providers.DefaultAwsRegionProviderChain; -import software.amazon.awssdk.utils.AttributeMap; +import software.amazon.awssdk.services.sts.auth.StsAssumeRoleCredentialsProvider; /** * Options used to configure Amazon Web Services specific options such as credentials and region. */ @Experimental(Kind.SOURCE_SINK) public interface AwsOptions extends PipelineOptions { - /** AWS region used by the AWS client. */ - @Description("AWS region used by the AWS client") + /** Region used to configure AWS service clients. */ + @Description("Region used by AWS service clients") @Default.InstanceFactory(AwsRegionFactory.class) - String getAwsRegion(); + Region getAwsRegion(); - void setAwsRegion(String value); + void setAwsRegion(Region region); /** Attempt to load default region. */ - class AwsRegionFactory implements DefaultValueFactory<@Nullable String> { + class AwsRegionFactory implements DefaultValueFactory<@Nullable Region> { @Override @Nullable - public String create(PipelineOptions options) { + public Region create(PipelineOptions options) { try { - return new DefaultAwsRegionProviderChain().getRegion().id(); + return new DefaultAwsRegionProviderChain().getRegion(); } catch (SdkClientException e) { return null; } } } - /** The AWS service endpoint used by the AWS client. */ - @Description("AWS service endpoint used by the AWS client") - String getEndpoint(); + /** Endpoint used to configure AWS service clients. */ + @Description("Endpoint used by AWS service clients") + URI getEndpoint(); Review comment: This is a breaking change to improve type safety. Considering endpoints are primarily used for testing purposes, there seems to be very little risk. ########## File path: sdks/java/io/amazon-web-services2/src/main/java/org/apache/beam/sdk/io/aws2/options/AwsOptions.java ########## @@ -95,50 +137,43 @@ public AwsCredentialsProvider create(PipelineOptions options) { } /** - * The client configuration instance that should be used to configure AWS service clients. Please - * note that the configuration deserialization only allows one to specify proxy settings. + * {@link ProxyConfiguration} used to configure AWS service clients. + * + * <p>Note, only the options shown in the example below are supported. <code>username</code> and + * <code>password</code> are optional. + * + * <p>Example: * - * <p>For example, to specify the proxy endpoint, username and password, specify the following: - * <code> - * --proxyConfiguration={ + * <pre>{@code --proxyConfiguration={ * "endpoint": "http://hostname:port", * "username": "username", * "password": "password" - * } - * </code> + * }}</pre> */ @Description( - "The proxy configuration instance that should be used to configure AWS service " - + "clients. Please note that the configuration deserialization only allows one to specify " - + "proxy settings. For example, to specify the proxy endpoint, username and password, " - + "specify the following: --proxyConfiguration={\"endpoint\":\"http://hostname:port\", \"username\":\"username\", \"password\":\"password\"}") + "The proxy configuration used to configure AWS service clients. Example: " + + "--proxyConfiguration={\"endpoint\":\"http://hostname:port\", \"username\":\"username\", \"password\":\"password\"}") ProxyConfiguration getProxyConfiguration(); void setProxyConfiguration(ProxyConfiguration value); /** - * The client configuration instance that should be used to configure AWS service clients. Please - * note that the configuration deserialization allows aws http client configuration settings. + * {@link HttpClientConfiguration} used to configure AWS service clients. * - * <p>For example, to set different timeout for aws client service : Note that all the below - * fields are optional, so only add those configurations that need to be set. <code> - * --attributeMap={ - * "connectionAcquisitionTimeout":"PT1000S", - * "connectionMaxIdleTime":"PT3000S", - * "connectionTimeout":"PT10000S", - * "socketTimeout":"PT600S", - * "maxConnections":"10", - * "socketTimeout":"PT5000SS" - * } - * </code> + * <p>Example: * - * @return + * <pre>{@code --httpClientConfiguration={"socketTimeout":1000, "maxConnections":10}}</pre> */ @Description( - "The attribute map instance that should be used to configure AWS http client configuration parameters." - + "Mentioned parameters are the available parameters that can be set. All above parameters are " - + "optional set only those that need custom changes.") - AttributeMap getAttributeMap(); + "The HTTP client configuration used to configure AWS service clients. Example: " + + "--httpClientConfiguration={\"socketTimeout\":1000,\"maxConnections\":10}") + HttpClientConfiguration getHttpClientConfiguration(); + + void setHttpClientConfiguration(HttpClientConfiguration value); + + @Description("Factory class to configure AWS client builders") + @Default.Class(ClientBuilderFactory.DefaultClientBuilder.class) + Class<? extends ClientBuilderFactory> getClientBuilderFactory(); - void setAttributeMap(AttributeMap attributeMap); Review comment: Replaced, but wasn't used at all previously ... -- 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: github-unsubscr...@beam.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org