bharadwaj-aditya commented on code in PR #37433:
URL: https://github.com/apache/beam/pull/37433#discussion_r2734783904
##########
sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessorTest.java:
##########
@@ -57,14 +57,14 @@ public void testCreateOnlyOnce() {
SpannerAccessor acc2 = SpannerAccessor.getOrCreate(config1);
SpannerAccessor acc3 = SpannerAccessor.getOrCreate(config1);
- acc1.close();
- acc2.close();
- acc3.close();
+ // acc1.close();
Review Comment:
Is this not required anymore ? If it is not, please remove it rather than
comment it out
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java:
##########
@@ -389,4 +393,20 @@ public SpannerConfig
withUsingPlainTextChannel(ValueProvider<Boolean> plainText)
public SpannerConfig withUsingPlainTextChannel(boolean plainText) {
return
withUsingPlainTextChannel(ValueProvider.StaticValueProvider.of(plainText));
}
+
+ /**
+ * @param waitForMinSessionsDuration
+ * @return {@link SpannerConfig}
+ *
+ * Sets the wait time for multiplexed session to be available while creating
a database client.
+ * Setting this will block the {@link
com.google.cloud.spanner.DatabaseClient} creation. By default,
+ * We will be setting 5 mins as minimum wait time.
+ */
+ public SpannerConfig
withWaitForMinSessionsDuration(ValueProvider<java.time.Duration>
waitForMinSessionsDuration) {
Review Comment:
naming nit: This is the wait for a session to be created. Min sessions seems
to suggest a certain number of sessions are being created.
Maybe call it waitForSessionCreationDuration or something ?
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java:
##########
@@ -265,6 +266,15 @@ static SpannerOptions buildSpannerOptions(SpannerConfig
spannerConfig) {
builder.setCredentials(credentials.get());
}
+ ValueProvider<java.time.Duration> waitForMinSessionsDuration =
spannerConfig.getWaitForMinSessionsDuration();
+ java.time.Duration waitDuration = java.time.Duration.ofMinutes(5);
Review Comment:
Please move this into a constant and add reasoning for the constant in the
docs.
--
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]
