pabloem commented on a change in pull request #15785:
URL: https://github.com/apache/beam/pull/15785#discussion_r745148763



##########
File path: 
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java
##########
@@ -57,6 +60,8 @@
 
   public abstract @Nullable ValueProvider<Duration> getMaxCumulativeBackoff();
 
+  public abstract @Nullable ValueProvider<RpcPriority> getRpcPriority();

Review comment:
       This does not have to be a ValueProvider argument, especially since it's 
not really exposed to users (they only use `withLowPriority` or 
`withHighPriority` to set this). You can make it directly `RpcPriority` type

##########
File path: 
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java
##########
@@ -172,4 +180,12 @@ public SpannerConfig 
withMaxCumulativeBackoff(ValueProvider<Duration> maxCumulat
   SpannerConfig withServiceFactory(ServiceFactory<Spanner, SpannerOptions> 
serviceFactory) {
     return toBuilder().setServiceFactory(serviceFactory).build();
   }
+
+  public SpannerConfig withRpcPriority(ValueProvider<RpcPriority> rpcPriority) 
{
+    return toBuilder().setRpcPriority(rpcPriority).build();
+  }

Review comment:
       If we don't make this a ValueProvider type, then we can also remove this 
whole method : )

##########
File path: 
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java
##########
@@ -57,6 +60,8 @@
 
   public abstract @Nullable ValueProvider<Duration> getMaxCumulativeBackoff();
 
+  public abstract @Nullable ValueProvider<RpcPriority> getRpcPriority();

Review comment:
       It seems that we have a default value for it, so it doesn't have to be 
`@Nullable` either, right?




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to