[GitHub] [beam] mosche commented on a change in pull request #16405: [BEAM-13587] Attempt to load AWS region from default provider chain in AwsOptions

[GitBox <gi...@apache.org>]

mosche commented on a change in pull request #16405:
URL: https://github.com/apache/beam/pull/16405#discussion_r777477157



##########
File path: sdks/java/io/amazon-web-services/src/main/java/org/apache/beam/sdk/io/aws/options/AwsOptions.java
##########
@@ -33,11 +34,23 @@
 
   /** AWS region used by the AWS client. */
   @Description("AWS region used by the AWS client")
-  @Validation.Required
+  @Default.InstanceFactory(AwsRegionFactory.class)
   String getAwsRegion();
 
   void setAwsRegion(String value);
 
+  /** Attempt to load default region. */
+  class AwsRegionFactory implements DefaultValueFactory<String> {
+    @Override
+    public String create(PipelineOptions options) {
+      try {
+        return new DefaultAwsRegionProviderChain().getRegion();
+      } catch (SdkClientException e) {
+        return null;

Review comment:
       No, this should be silent ... if no region is found, the option should be `null` as it is at the moment in this case. Same regarding the NPE, this just sets a region automatically if available. Otherwise the behavior is unchanged.
   Btw, check this comment in the AWS SDK itself:
   ```
   // Note: This is a bug in the provider chain. The chain should return null when no region is found according to
    // the interface, but an exception is thrown  here instead. This class is used in too many places to change now.
   // TODO: In 2.0, be sure this bug does not carry through.
    throw new SdkClientException("Unable to load region information from any provider in the chain"
   ```




-- 
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-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org