You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2020/03/29 09:40:27 UTC

[GitHub] [beam] iemejia opened a new pull request #11258: [BEAM-9571] Add withMongoClientProvider to MongoDbIO

iemejia opened a new pull request #11258: [BEAM-9571] Add withMongoClientProvider to MongoDbIO
URL: https://github.com/apache/beam/pull/11258
 
 
   This introduces the option so users can configure their MongoClients at will, as discussed in #11186 PTAL
   R: @alexvanboxel 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] iemejia commented on issue #11258: [BEAM-9571] Add withMongoClientProvider to MongoDbIO

Posted by GitBox <gi...@apache.org>.
iemejia commented on issue #11258: [BEAM-9571] Add withMongoClientProvider to MongoDbIO
URL: https://github.com/apache/beam/pull/11258#issuecomment-606521455
 
 
   Thanks for the review @alexvanboxel You have a point about the javadoc. I will add that.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] alexvanboxel commented on a change in pull request #11258: [BEAM-9571] Add withMongoClientProvider to MongoDbIO

Posted by GitBox <gi...@apache.org>.
alexvanboxel commented on a change in pull request #11258: [BEAM-9571] Add withMongoClientProvider to MongoDbIO
URL: https://github.com/apache/beam/pull/11258#discussion_r400705757
 
 

 ##########
 File path: sdks/java/io/mongodb/src/main/java/org/apache/beam/sdk/io/mongodb/MongoDbIO.java
 ##########
 @@ -191,7 +193,29 @@ private MongoDbIO() {}
       abstract Builder setQueryFn(
           SerializableFunction<MongoCollection<Document>, MongoCursor<Document>> queryBuilder);
 
-      abstract Read build();
 
 Review comment:
   Shouldn't this be a rename as well? It looks like a removal.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] alexvanboxel commented on a change in pull request #11258: [BEAM-9571] Add withMongoClientProvider to MongoDbIO

Posted by GitBox <gi...@apache.org>.
alexvanboxel commented on a change in pull request #11258: [BEAM-9571] Add withMongoClientProvider to MongoDbIO
URL: https://github.com/apache/beam/pull/11258#discussion_r410057736
 
 

 ##########
 File path: sdks/java/io/mongodb/src/main/java/org/apache/beam/sdk/io/mongodb/MongoDbIO.java
 ##########
 @@ -843,22 +843,39 @@ private MongoClient createClient(Read spec) {
      */
     public Write withUri(String uri) {
       checkArgument(uri != null, "uri can not be null");
-      return builder().setUri(uri).build();
+      Write spec = toBuilder().setUri(uri).build();
+      return withMongoClientProvider(
+              DefaultMongoClientProvider.of(
+                  spec.uri(),
+                  spec.maxConnectionIdleTime(),
+                  spec.sslEnabled(),
+                  spec.sslInvalidHostNameAllowed(),
+                  spec.ignoreSSLCertificate()))
+          .toBuilder()
+          .setUri(uri)
+          .build();
     }
 
     /** Sets the maximum idle time for a pooled connection. */
     public Write withMaxConnectionIdleTime(int maxConnectionIdleTime) {
-      return builder().setMaxConnectionIdleTime(maxConnectionIdleTime).build();
+      return toBuilder().setMaxConnectionIdleTime(maxConnectionIdleTime).build();
     }
 
     /** Enable ssl for connection. */
     public Write withSSLEnabled(boolean sslEnabled) {
-      return builder().setSslEnabled(sslEnabled).build();
+      return toBuilder().setSslEnabled(sslEnabled).build();
     }
 
     /** Enable invalidHostNameAllowed for ssl for connection. */
     public Write withSSLInvalidHostNameAllowed(boolean invalidHostNameAllowed) {
-      return builder().setSslInvalidHostNameAllowed(invalidHostNameAllowed).build();
+      return toBuilder().setSslInvalidHostNameAllowed(invalidHostNameAllowed).build();
+    }
+
+    /** Sets a MongoClient provider. */
+    public Write withMongoClientProvider(
 
 Review comment:
   Maybe we should rename this to withClientProvider, the we can use this pattern in other IO's and make it as part of a IO style guide to make them more consistent. WDYT?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] iemejia removed a comment on issue #11258: [BEAM-9571] Add withMongoClientProvider to MongoDbIO

Posted by GitBox <gi...@apache.org>.
iemejia removed a comment on issue #11258: [BEAM-9571] Add withMongoClientProvider to MongoDbIO
URL: https://github.com/apache/beam/pull/11258#issuecomment-605650970
 
 
   Run Java PreCommit

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] iemejia commented on a change in pull request #11258: [BEAM-9571] Add withMongoClientProvider to MongoDbIO

Posted by GitBox <gi...@apache.org>.
iemejia commented on a change in pull request #11258: [BEAM-9571] Add withMongoClientProvider to MongoDbIO
URL: https://github.com/apache/beam/pull/11258#discussion_r399772016
 
 

 ##########
 File path: sdks/java/io/mongodb/src/main/java/org/apache/beam/sdk/io/mongodb/MongoDbIO.java
 ##########
 @@ -166,7 +165,7 @@ private MongoDbIO() {}
 
     abstract SerializableFunction<MongoCollection<Document>, MongoCursor<Document>> queryFn();
 
-    abstract Builder builder();
+    abstract Builder toBuilder();
 
 Review comment:
   This rename is slightly unrelated to this PR but an improvement to be consistent with [AutoValue conventions](https://github.com/google/auto/blob/master/value/userguide/builders-howto.md#to_builder).

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] iemejia commented on a change in pull request #11258: [BEAM-9571] Add withMongoClientProvider to MongoDbIO

Posted by GitBox <gi...@apache.org>.
iemejia commented on a change in pull request #11258: [BEAM-9571] Add withMongoClientProvider to MongoDbIO
URL: https://github.com/apache/beam/pull/11258#discussion_r399772016
 
 

 ##########
 File path: sdks/java/io/mongodb/src/main/java/org/apache/beam/sdk/io/mongodb/MongoDbIO.java
 ##########
 @@ -166,7 +165,7 @@ private MongoDbIO() {}
 
     abstract SerializableFunction<MongoCollection<Document>, MongoCursor<Document>> queryFn();
 
-    abstract Builder builder();
+    abstract Builder toBuilder();
 
 Review comment:
   This rename is slightly unreated to this PR but an improvement to be consistent with [AutoValue conventions](https://github.com/google/auto/blob/master/value/userguide/builders-howto.md#to_builder).

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] iemejia commented on a change in pull request #11258: [BEAM-9571] Add withMongoClientProvider to MongoDbIO

Posted by GitBox <gi...@apache.org>.
iemejia commented on a change in pull request #11258: [BEAM-9571] Add withMongoClientProvider to MongoDbIO
URL: https://github.com/apache/beam/pull/11258#discussion_r400785847
 
 

 ##########
 File path: sdks/java/io/mongodb/src/main/java/org/apache/beam/sdk/io/mongodb/MongoDbIO.java
 ##########
 @@ -191,7 +193,29 @@ private MongoDbIO() {}
       abstract Builder setQueryFn(
           SerializableFunction<MongoCollection<Document>, MongoCursor<Document>> queryBuilder);
 
-      abstract Read build();
 
 Review comment:
   This one was not renamed, it changed because now it has a default implementation (see below in line 208). The default implementation sets a provider given the respective parameters.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] iemejia commented on a change in pull request #11258: [BEAM-9571] Add withMongoClientProvider to MongoDbIO

Posted by GitBox <gi...@apache.org>.
iemejia commented on a change in pull request #11258: [BEAM-9571] Add withMongoClientProvider to MongoDbIO
URL: https://github.com/apache/beam/pull/11258#discussion_r400785847
 
 

 ##########
 File path: sdks/java/io/mongodb/src/main/java/org/apache/beam/sdk/io/mongodb/MongoDbIO.java
 ##########
 @@ -191,7 +193,29 @@ private MongoDbIO() {}
       abstract Builder setQueryFn(
           SerializableFunction<MongoCollection<Document>, MongoCursor<Document>> queryBuilder);
 
-      abstract Read build();
 
 Review comment:
   This one was not renamed, it changed because now it has a default implementation (see below in line 208). The default implementation sets a provider given the respective parameters.
   Oh I think I see where the confusion was, the rename was from `builder()` to `toBuilder()` to follow AutoValue's recommendations and be consistent with most of the other IOs.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] iemejia commented on issue #11258: [BEAM-9571] Add withMongoClientProvider to MongoDbIO

Posted by GitBox <gi...@apache.org>.
iemejia commented on issue #11258: [BEAM-9571] Add withMongoClientProvider to MongoDbIO
URL: https://github.com/apache/beam/pull/11258#issuecomment-605650970
 
 
   Run Java PreCommit

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services