[ZEPPELIN-5991] Adding Support of DocumentDb and Mongo Atlas for MongoDB interpreter #4706
Conversation
Add Support for AWS documentDB and Atlas
removing globalThis as it generate errors
adding doc
mithun12000
changed the title
|
Thank you for your first contribution :-) I'll review this PR soon |
|
|
||
| /* adding support for protocal like mongodb+srv for atlas cluster*/ | ||
| String mongoProtocol = getProperty("mongo.server.protocol", "mongodb"); | ||
| if ("mongodb".equalsIgnoreCase(mongoProtocol)){ |
There was a problem hiding this comment.
Could you please check your indentation settings?
There was a problem hiding this comment.
Sure will update indentation.
| if ("mongodb".equalsIgnoreCase(mongoProtocol)){ | ||
| dbAddress = getProperty("mongo.server.host") + ":" + getProperty("mongo.server.port"); | ||
| }else{ | ||
| dbAddress = mongoProtocol +"://"+ getProperty("mongo.server.host"); |
There was a problem hiding this comment.
If we have a protocol value, does it mean mongo.server.port? Otherwise, don't some protocols need port value? Could you please verify it?
There was a problem hiding this comment.
if protocol is mongodb then it will use mongo.server.port as many provider use different port for mongo / documentDb or similar service.
but i never seen mongodb+srv protocol is been used with different port.
There was a problem hiding this comment.
Thank you for the response. BTW, for the conservative way, could you please add the code for port availability? I also don't know if it's really needed but I think it would be better to add it anyway. WDYT?
There was a problem hiding this comment.
Ok will update the code as requested.
| @@ -117,6 +125,67 @@ public InterpreterResult interpret(String script, InterpreterContext context) { | |||
| executor.setWatchdog(new ExecuteWatchdog(commandTimeout)); | |||
|
|
|||
| final CommandLine cmdLine = CommandLine.parse(getProperty("mongo.shell.path")); | |||
| /* added support for API versions */ | |||
| String apiVersion = getProperty("mongo.server.api.version", ""); | |||
| if (!"".equalsIgnoreCase(apiVersion)){ | |||
There was a problem hiding this comment.
nit: StringUtils.isBlank(...)?
|
|
||
| /* adding support for SSL for and TLS for documentDB */ | ||
| String runWithSSL = getProperty("mongo.server.ssl.enabled", "false"); | ||
| if ("true".equalsIgnoreCase(runWithSSL)) |
There was a problem hiding this comment.
nit: Boolean.valueOf(...) or Boolean.parseBoolean(...)?
|
I left some comments. Could you please check it? Moreover, I think it would be good to have some tests for your contribution. |
What is this PR for?
Adding Support of DocumentDb and Mongo Atlas for MongoDB interpreter.
DocumentDB in AWS required SSL/ TSL connection which was not supported.
MongoDB Atlas required special protocal to connect
mongodb+srvWhat type of PR is it?
Improvement
Todos
What is the Jira issue?
ZEPPELIN-5991
How should this be tested?
Questions: