-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8148/#review13864
-----------------------------------------------------------



core/src/main/java/org/apache/hcatalog/mapreduce/HCatInputFormat.java
<https://reviews.apache.org/r/8148/#comment29641>

    minor change request: Can we have this similar to setProperties ? 
    Preconditions.checkNotNull(filter, "required argument 'filter' is null");


- Vandana Ayyalasomayajula


On Nov. 29, 2012, 1:46 a.m., Travis Crawford wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8148/
> -----------------------------------------------------------
> 
> (Updated Nov. 29, 2012, 1:46 a.m.)
> 
> 
> Review request for hcatalog.
> 
> 
> Description
> -------
> 
> Currently we expose the following interface to use HCatInputFormat:
> 
> """
> HCatInputFormat.setInput(job, InputJobInfo.create(dbName, inputTableName, 
> null));
> """
> 
> Notice how InputJobInfo is exposed directly to end-users. InputJobInfo should 
> be treated as an implementation detail, and the current API should be 
> deprecated and later removed.
> 
> This change is my first pass at deprecating the above. Feedback would be 
> appreciated. In particular, I'm interested in what method signatures we 
> should provide people. At present we need the following:
> 
> Configuration - required
> database name - required
> table name - required
> filter - optional
> properties - optional
> 
> To keep things simple I depercated everything except a single constructor 
> that takes those above arguments.
> 
> Questions:
> * its cumbersome for users to say job.getConfiguration instead of passing in 
> a job - should we provide that convenience?
> * filter is optional - should we provide a method that automatically nulls 
> out?
> * properties is optional and rarely used (I believe only in the hbase storage 
> handler) - should we provide a method that automatically sets to null?
> 
> 
> This addresses bug HCATALOG-527.
>     https://issues.apache.org/jira/browse/HCATALOG-527
> 
> 
> Diffs
> -----
> 
>   
> core/src/main/java/org/apache/hcatalog/data/transfer/impl/HCatInputFormatReader.java
>  cf9bba2 
>   core/src/main/java/org/apache/hcatalog/mapreduce/HCatInputFormat.java 
> 7a7446f 
>   core/src/main/java/org/apache/hcatalog/mapreduce/InitializeInput.java 
> e91ddc7 
>   core/src/main/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 8779c5e 
>   core/src/test/java/org/apache/hcatalog/data/TestReaderWriter.java 18d368f 
>   core/src/test/java/org/apache/hcatalog/mapreduce/HCatMapReduceTest.java 
> 74c4888 
>   core/src/test/java/org/apache/hcatalog/mapreduce/TestHCatInputFormat.java 
> a6b381e 
>   hcatalog-pig-adapter/src/main/java/org/apache/hcatalog/pig/HCatLoader.java 
> 5a4d400 
>   
> storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/TestHBaseBulkOutputFormat.java
>  3875185 
>   
> storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/TestHBaseDirectOutputFormat.java
>  81a3099 
>   
> storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/TestHBaseInputFormat.java
>  78724a1 
>   
> storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/TestSnapshots.java 
> e07bf46 
> 
> Diff: https://reviews.apache.org/r/8148/diff/
> 
> 
> Testing
> -------
> 
> I have not run tests yet as I'm soliciting feedback on the interface change 
> at this time. Will run tests and our internal integration tests before 
> posting a final version.
> 
> 
> Thanks,
> 
> Travis Crawford
> 
>

Reply via email to