----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8148/#review13694 -----------------------------------------------------------
Here is my thinking: >> * its cumbersome for users to say job.getConfiguration instead of passing in >> a job - should we provide that convenience? I think not, actually it provides more flexibility cuz sometimes user only get configuration not job. >> * filter is optional - should we provide a method that automatically nulls >> out? Sure, why not >> * properties is optional and rarely used (I believe only in the hbase >> storage handler) - should we provide a method that automatically sets to >> null? Sure, why not - Daniel Dai On Nov. 21, 2012, 1:01 a.m., Travis Crawford wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8148/ > ----------------------------------------------------------- > > (Updated Nov. 21, 2012, 1:01 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/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 > 4bdb7c3 > > 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 > >
