mcvsubbu commented on a change in pull request #4959: Add schemaFile as option
in AddTableCommand
URL: https://github.com/apache/incubator-pinot/pull/4959#discussion_r363109707
##########
File path:
pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/AddTableCommand.java
##########
@@ -102,33 +112,51 @@ public AddTableCommand setExecute(boolean exec) {
return this;
}
- public boolean execute(JsonNode node)
+ public boolean sendTableCreationRequest(JsonNode node)
throws IOException {
- if (_controllerHost == null) {
- _controllerHost = NetUtil.getHostAddress();
- }
- _controllerAddress = "http://"; + _controllerHost + ":" + _controllerPort;
+ String res = AbstractBaseAdminCommand
+
.sendPostRequest(ControllerRequestURLBuilder.baseUrl(_controllerAddress).forTableCreate(),
node.toString());
+ LOGGER.info(res);
+ return res.contains("succesfully added");
+ }
+ @Override
+ public boolean execute()
+ throws Exception {
if (!_exec) {
LOGGER.warn("Dry Running Command: " + toString());
LOGGER.warn("Use the -exec option to actually execute the command.");
return true;
}
- LOGGER.info("Executing command: " + toString());
- String res = AbstractBaseAdminCommand
-
.sendPostRequest(ControllerRequestURLBuilder.baseUrl(_controllerAddress).forTableCreate(),
node.toString());
+ if (_controllerHost == null) {
+ _controllerHost = NetUtil.getHostAddress();
+ }
+ _controllerAddress = "http://"; + _controllerHost + ":" + _controllerPort;
- LOGGER.info(res);
- if (res.contains("succesfully added")) {
- return true;
+ LOGGER.info("Executing command: " + toString());
+ // Backward compatible
+ if (_schemaFile != null) {
+ File schemaFile = new File(_schemaFile);
+ if (!schemaFile.exists()) {
+ throw new FileNotFoundException("schema file does not exist: " +
_schemaFile);
+ }
+
+ Schema schema = Schema.fromFile(schemaFile);
Review comment:
You may get an exception here if the schema file is illegal or incorrect
format. Not sure what the exception looks like, but it may be better to
explicitly field any exception from this call and wrap it into an exception
that contains the file name, e.g. "Incorrect schema specification in
<schemafilepath>". This way, you can also avoid checking if the file existance
in line 141, and combine all exceptions here.
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
