You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by GitBox <gi...@apache.org> on 2021/10/27 11:54:25 UTC

[GitHub] [drill] luocooong opened a new pull request #2352: DRILL-8021: Add the srv protocol support for the mongo storage

luocooong opened a new pull request #2352:
URL: https://github.com/apache/drill/pull/2352


   # [DRILL-8021](https://issues.apache.org/jira/browse/DRILL-8021): Add the srv protocol support for the mongo storage
   
   ## Description
   To support the `srv` protocol :
   ```json
   {
     "type": "mongo",
     "connection": "mongodb+srv://xxx",
   }
   ```
   
   [DNS Seed List Connection Format](https://docs.mongodb.com/manual/reference/connection-string/#dns-seed-list-connection-format)
   
   ## Documentation
   N/A
   
   ## Testing
   Manual tested.
   
   Successful results :
   <pre>
   apache drill> show databases;
   +---------------------+
   |     SCHEMA_NAME     |
   +---------------------+
   | cp.default          |
   | dfs.default         |
   | dfs.root            |
   | dfs.tmp             |
   | information_schema  |
   | mongo_srv.admin     |
   | mongo_srv.config    |
   | mongo_srv.local     |
   | mongo_srv.test      |
   +---------------------+
   28 rows selected (3.849 seconds)
   
   apache drill> select * from mongo_srv.test.test;
   +--------------------------------------+-------------------------+
   |                 _id                  |          date           |
   +--------------------------------------+-------------------------+
   | a"j\xC3;)\xFFc\x06\xB3j\xA9          | 2021-08-22 23:18:27.258 |
   | a"j\xC3;)\xFFc\x06\xB3j\xAA          | 2021-08-22 23:18:27.274 |
   | a"j\xC3;)\xFFc\x06\xB3j\xAB          | 2021-08-22 21:57:14.123 |
   | a"j\xC3;)\xFFc\x06\xB3j\xAC          | 2021-08-22 21:57:14.0   |
   | a%\xE4\xD2\xE8t\x86W\xED\xE6\x10\x1A | 2019-08-12 01:54:14.692 |
   +--------------------------------------+-------------------------+
   5 rows selected (3.409 seconds)
   </pre>
   


-- 
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: dev-unsubscribe@drill.apache.org

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



[GitHub] [drill] cgivre commented on a change in pull request #2352: DRILL-8021: Add the srv protocol support for the mongo storage

Posted by GitBox <gi...@apache.org>.
cgivre commented on a change in pull request #2352:
URL: https://github.com/apache/drill/pull/2352#discussion_r737391537



##########
File path: contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoStoragePlugin.java
##########
@@ -195,6 +199,18 @@ public MongoClient getClient() {
     return getClient(addresses);
   }
 
+  public synchronized MongoClient getClientWithSrvProtocol() {
+    MongoClientSettings settings = MongoClientSettings.builder()
+        .applyConnectionString(clientURI)
+        .build();
+    MongoClient client = MongoClients.create(settings);
+    ServerAddress serverAddress = new ServerAddress(clientURI.getHosts().get(0));
+    String userName = clientURI.getCredential().getUserName();

Review comment:
       @luocooong 
   Thanks for the rapid response on this.  I have a minor suggestion.  We should include support for credentials coming from a credential provider, not just hard coded creds in the URL.  I think you can reuse some code below. 
   Thanks again!




-- 
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: dev-unsubscribe@drill.apache.org

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



[GitHub] [drill] cgivre merged pull request #2352: DRILL-8021: Add the srv protocol support for the mongo storage

Posted by GitBox <gi...@apache.org>.
cgivre merged pull request #2352:
URL: https://github.com/apache/drill/pull/2352


   


-- 
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: dev-unsubscribe@drill.apache.org

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



[GitHub] [drill] cgivre commented on a change in pull request #2352: DRILL-8021: Add the srv protocol support for the mongo storage

Posted by GitBox <gi...@apache.org>.
cgivre commented on a change in pull request #2352:
URL: https://github.com/apache/drill/pull/2352#discussion_r737833379



##########
File path: contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoStoragePlugin.java
##########
@@ -195,6 +199,18 @@ public MongoClient getClient() {
     return getClient(addresses);
   }
 
+  public synchronized MongoClient getClientWithSrvProtocol() {
+    MongoClientSettings settings = MongoClientSettings.builder()
+        .applyConnectionString(clientURI)
+        .build();
+    MongoClient client = MongoClients.create(settings);
+    ServerAddress serverAddress = new ServerAddress(clientURI.getHosts().get(0));
+    String userName = clientURI.getCredential().getUserName();

Review comment:
       In that case LGTM +1




-- 
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: dev-unsubscribe@drill.apache.org

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



[GitHub] [drill] luocooong commented on a change in pull request #2352: DRILL-8021: Add the srv protocol support for the mongo storage

Posted by GitBox <gi...@apache.org>.
luocooong commented on a change in pull request #2352:
URL: https://github.com/apache/drill/pull/2352#discussion_r737409995



##########
File path: contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoStoragePlugin.java
##########
@@ -195,6 +199,18 @@ public MongoClient getClient() {
     return getClient(addresses);
   }
 
+  public synchronized MongoClient getClientWithSrvProtocol() {
+    MongoClientSettings settings = MongoClientSettings.builder()
+        .applyConnectionString(clientURI)
+        .build();
+    MongoClient client = MongoClients.create(settings);
+    ServerAddress serverAddress = new ServerAddress(clientURI.getHosts().get(0));
+    String userName = clientURI.getCredential().getUserName();

Review comment:
       Good practices. But we have done it. The credentials in the `clientURI` variable were processed during the construction period, and the `clientURI` is only available in the memory.  See that :
   https://github.com/apache/drill/blob/b6da35ece5a278a5ca72ecc33bafc98ba8f861f6/contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoStoragePlugin.java#L80-L81




-- 
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: dev-unsubscribe@drill.apache.org

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



[GitHub] [drill] luocooong commented on pull request #2352: DRILL-8021: Add the srv protocol support for the mongo storage

Posted by GitBox <gi...@apache.org>.
luocooong commented on pull request #2352:
URL: https://github.com/apache/drill/pull/2352#issuecomment-953676634


   @cgivre Done. I made a small refactoring and fixed a issue that statistics of connections.


-- 
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: dev-unsubscribe@drill.apache.org

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



[GitHub] [drill] luocooong commented on pull request #2352: DRILL-8021: Add the srv protocol support for the mongo storage

Posted by GitBox <gi...@apache.org>.
luocooong commented on pull request #2352:
URL: https://github.com/apache/drill/pull/2352#issuecomment-953379638


   @cgivre Thanks for your review.


-- 
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: dev-unsubscribe@drill.apache.org

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



[GitHub] [drill] cgivre commented on pull request #2352: DRILL-8021: Add the srv protocol support for the mongo storage

Posted by GitBox <gi...@apache.org>.
cgivre commented on pull request #2352:
URL: https://github.com/apache/drill/pull/2352#issuecomment-953495582


   @luocooong Are we ready to merge this?  Why did you mark this as blocked?


-- 
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: dev-unsubscribe@drill.apache.org

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