[GitHub] [drill] luocooong commented on a change in pull request #2131: DRILL-7830: Add XML as Supported Format for HTTP Plugin

[GitBox <gi...@apache.org>]

luocooong commented on a change in pull request #2131:
URL: https://github.com/apache/drill/pull/2131#discussion_r549655406



##########
File path: contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpScanBatchCreator.java
##########
@@ -99,6 +99,8 @@ public void bind(ManagedScanFramework framework) { }
       if (count++ == 0) {
         if (inputType.equalsIgnoreCase("csv")) {
           return new HttpCSVBatchReader(subScan);
+        } else if (inputType.equalsIgnoreCase("xml")) {

Review comment:
       Could 'csv' and 'xml' reuse the final static variables recommended above?

##########
File path: contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpApiConfig.java
##########
@@ -131,6 +133,8 @@ public HttpApiConfig(@JsonProperty("url") String url,
 
     this.inputType = inputType == null
       ? "json" : inputType.trim().toLowerCase();

Review comment:
       It is recommended to define "json" as a final static variable.




----------------------------------------------------------------
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