KazydubB commented on a change in pull request #1635: DRILL-7021: HTTPD Throws 
NPE and Doesn't Recognize Timeformat
URL: https://github.com/apache/drill/pull/1635#discussion_r264604328
 
 

 ##########
 File path: 
exec/java-exec/src/main/java/org/apache/drill/exec/store/httpd/HttpdLogFormatPlugin.java
 ##########
 @@ -54,74 +52,26 @@
 import org.apache.hadoop.mapred.Reporter;
 import org.apache.hadoop.mapred.TextInputFormat;
 
-import com.fasterxml.jackson.annotation.JsonTypeName;
 import org.apache.drill.shaded.guava.com.google.common.collect.Lists;
 import org.apache.drill.shaded.guava.com.google.common.collect.Maps;
+
 import java.util.Map;
+
 import org.apache.drill.exec.store.RecordReader;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-public class HttpdLogFormatPlugin extends 
EasyFormatPlugin<HttpdLogFormatPlugin.HttpdLogFormatConfig> {
+public class HttpdLogFormatPlugin extends 
EasyFormatPlugin<HttpdLogFormatConfig> {
 
   private static final Logger LOG = 
LoggerFactory.getLogger(HttpdLogFormatPlugin.class);
   private static final String PLUGIN_EXTENSION = "httpd";
   private static final int VECTOR_MEMORY_ALLOCATION = 4095;
 
   public HttpdLogFormatPlugin(final String name, final DrillbitContext 
context, final Configuration fsConf,
-      final StoragePluginConfig storageConfig, final HttpdLogFormatConfig 
formatConfig) {
+                              final StoragePluginConfig storageConfig, final 
HttpdLogFormatConfig formatConfig) {
 
     super(name, context, fsConf, storageConfig, formatConfig, true, false, 
true, true,
-        Lists.newArrayList(PLUGIN_EXTENSION), PLUGIN_EXTENSION);
-  }
-
-  /**
-   * This class is a POJO to hold the configuration for the HttpdLogFormat 
Parser. This is automatically
-   * serialized/deserialized from JSON format.
-   */
-  @JsonTypeName(PLUGIN_EXTENSION) @JsonInclude(JsonInclude.Include.NON_DEFAULT)
-  public static class HttpdLogFormatConfig implements FormatPluginConfig {
-
-    public String logFormat;
-    public String timestampFormat;
-
-    /**
-     * @return the logFormat
-     */
-    public String getLogFormat() {
-      return logFormat;
-    }
-
-    /**
-     * @return the timestampFormat
-     */
-    public String getTimestampFormat() {
-      return timestampFormat;
-    }
-
-    @Override
-    public int hashCode() {
-      int result = logFormat != null ? logFormat.hashCode() : 0;
-      result = 31 * result + (timestampFormat != null ? 
timestampFormat.hashCode() : 0);
-      return result;
-    }
-
-    @Override
-    public boolean equals(Object o) {
-      if (this == o) {
-        return true;
-      }
-      if (o == null || getClass() != o.getClass()) {
-        return false;
-      }
-
-      HttpdLogFormatConfig that = (HttpdLogFormatConfig) o;
-
-      if (logFormat != null ? !logFormat.equals(that.logFormat) : 
that.logFormat != null) {
-        return false;
-      }
-      return timestampFormat != null ? 
timestampFormat.equals(that.timestampFormat) : that.timestampFormat == null;
-    }
+            Lists.newArrayList(PLUGIN_EXTENSION), PLUGIN_EXTENSION);
 
 Review comment:
   It is clearer that the list will contain one element only and won't be 
modifiable. Also no new backing array will be created.
   
   (Plus no Guava dependency will be used (though that's debatable whether this 
should be the case).)

----------------------------------------------------------------
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:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to