----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25586/#review53215 -----------------------------------------------------------
Went through it once... A few details: 1. I can't seem to find the "Transferrable" class. Is it possible that you have previous commits that are missing? 2. We should rename "Transferrable" to "Transferable". 3. Kill off the extra white space! common/src/main/java/org/apache/sqoop/model/MSubmission.java <https://reviews.apache.org/r/25586/#comment92713> Can we leave the schema stuff alone for SQOOP-1378? core/src/main/java/org/apache/sqoop/framework/JobManager.java <https://reviews.apache.org/r/25586/#comment92716> Rename to "fromTransferable" and "toTransferable". - Abraham Elmahrek On Sept. 12, 2014, 5:42 p.m., Veena Basavaraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25586/ > ----------------------------------------------------------- > > (Updated Sept. 12, 2014, 5:42 p.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-1496 > https://issues.apache.org/jira/browse/SQOOP-1496 > > > Repository: sqoop-SQOOP-1367 > > > Description > ------- > > renamed BaseCallbacks to Transferrable ( Since the refactoring to From/to it > really does not make sense to call them "baseCallbacks", everything in the > pluggable/API model is a callback, and from/to are no different) > > renamed SubmissionRequest to JobRequest (See JIRA that explains why this > seems more intuitive ) , I am keeping the SubmissionEngine terminology as it > is, and will revisit at a later point when a new submission/execution engine > will be supported > fixed #2 ( execution engine now creates a job request and a submission > request. submission engine submits the job request)... uuf! > fixed #1 ( refactored to smaller methods, so that the code is readable and > testable) > fixed #3 ( added a unit test for the job manager submit call) > > Aside > remove unused imports from the some classes > > more clean up > - rename the fallouts from the from/to refactoring to reflect the from/to > schema. ( this overlaps with some of the work in > https://issues.apache.org/jira/browse/SQOOP-1378 > https://reviews.apache.org/r/25180/ from Gwen still pending commit_ > > > Diffs > ----- > > common/src/main/java/org/apache/sqoop/json/SubmissionBean.java 61d6576 > common/src/main/java/org/apache/sqoop/model/MSubmission.java 1edd6ee > common/src/test/java/org/apache/sqoop/json/TestSubmissionBean.java d87655e > > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/CSVIntermediateDataFormat.java > 1e8ab52 > > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/IntermediateDataFormat.java > 91b594e > > connector/connector-sdk/src/test/java/org/apache/sqoop/connector/idf/CSVIntermediateDataFormatTest.java > df6d30f > core/src/main/java/org/apache/sqoop/framework/ExecutionEngine.java 96ec148 > core/src/main/java/org/apache/sqoop/framework/JobManager.java b1b37f6 > core/src/main/java/org/apache/sqoop/framework/SubmissionEngine.java 3c0f6eb > core/src/main/java/org/apache/sqoop/framework/SubmissionRequest.java > bf3f785 > core/src/test/java/org/apache/sqoop/framework/TestSubmissionRequest.java > 3078ed2 > > execution/mapreduce/src/main/java/org/apache/sqoop/execution/mapreduce/MRSubmissionRequest.java > 32d598c > > execution/mapreduce/src/main/java/org/apache/sqoop/execution/mapreduce/MapreduceExecutionEngine.java > b05954b > shell/src/main/java/org/apache/sqoop/shell/utils/SubmissionDisplayer.java > 6dbd870 > spi/src/main/java/org/apache/sqoop/job/etl/CallbackBase.java 59a9457 > spi/src/main/java/org/apache/sqoop/job/etl/From.java 9b8d76f > spi/src/main/java/org/apache/sqoop/job/etl/To.java a791945 > > submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java > bfa6958 > > Diff: https://reviews.apache.org/r/25586/diff/ > > > Testing > ------- > > unit/ integration tests pass > > if this patch is hard to follow, since I have split the jobmanager submit > into like many smaller methods, please feel free to apply the patch on a > branch and take a look > > > Thanks, > > Veena Basavaraj > >
