[GitHub] [hudi] pratyakshsharma commented on a change in pull request #1648: [HUDI-916]: added support for multiple input formats in TimestampBasedKeyGenerator

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

pratyakshsharma commented on a change in pull request #1648:
URL: https://github.com/apache/hudi/pull/1648#discussion_r446538524



##########
File path: hudi-spark/src/main/java/org/apache/hudi/keygen/parser/HoodieDateTimeParser.java
##########
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.keygen.parser;
+
+import org.joda.time.DateTimeZone;
+import org.joda.time.format.DateTimeFormatter;
+
+public interface HoodieDateTimeParser {
+
+  /**
+   * Returns the output date format in which the partition paths will be created for the hudi dataset.
+   * @return
+   */
+  String getOutputDateFormat();

Review comment:
       The purpose of not passing any params here is to actually use the parameters defined in TimestampBasedKeyGenerator.Config class. I want to have that Config class as the only place for having all the config parameters for timestamp based key generation and I have actually used those config parameters in my implementation as well. But I do not have any strong reason for not passing any parameters around as part of signature of all functions in HoodieDateTimeParser interface. The other reason is we cannot enforce the usage of parameter by passing in the function as someone might still want to change the code and they can change the interface contract itself at a later point of time. I guess we can take care of config parameters getting used as part of code review itself :) 
   
   Please let me know your thoughts on this. @nsivabalan 




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