kishoreg commented on a change in pull request #4866: Removing segment 
generation config from RecordReader interface
URL: https://github.com/apache/incubator-pinot/pull/4866#discussion_r351476621
 
 

 ##########
 File path: 
pinot-core/src/main/java/org/apache/pinot/core/data/readers/RecordReader.java
 ##########
 @@ -34,10 +34,16 @@
 public interface RecordReader extends Closeable {
 
   /**
-   * Initializes the record reader when needed
+   * initializing recordreader with inputpath, schema and recordreader config. 
<br/>
+   * The implementation can chose to ignore one or more of these parameters 
and handle null gracefully <br/>
+   *
+   * @param inputPath absolute path to the file/directory
+   * @param schema Pinot Schema associated with the table
+   * @param recordReaderConfig config for the reader specific to the format. 
e.g. delimiter for csv format etc
+   * @throws Exception if the arguments are invalid
    */
-  void init(SegmentGeneratorConfig segmentGeneratorConfig)
-      throws IOException;
+  void init(@Nullable String inputPath, @Nullable Schema schema, @Nullable 
RecordReaderConfig recordReaderConfig)
 
 Review comment:
   > This config is for record readers with extra settings (e.g. CSV 
delimiter), which can be null. Input path and schema are required for all 
record readers
   
   Not really, some of them take GenericRows. There are too many 
implementations for recordReader: used in testing, segment conversion, Minion 
etc. Most of the readers get what they want in their constructors today. 

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

Reply via email to