----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23531/#review48096 -----------------------------------------------------------
client/src/main/java/org/apache/sqoop/client/SqoopClient.java <https://reviews.apache.org/r/23531/#comment84396> total nitpicking, but txid reads too much like "transaction id" to me. I actually spent time wondering why would be pass transaction id to "getConnection..." I'm prefer toXid and fromXid for extra readablity. Another note: I'd prefer not to use IDs at all in our public interfaces because: * Users shouldn't know or care about them * IDs can change at some points (upgrade, migration) which may break customer's code. This is a big change (since we'll need to enforce name uniqueness), and I'm not sure if we want to tackle it here or in separate JIRA. common/src/main/java/org/apache/sqoop/model/MJob.java <https://reviews.apache.org/r/23531/#comment84398> Another nitpick: The interface breaks rhythm... We have: from,to,from,to,from,framework,to I'd prefer: from,to,from,to,from,to,framework - Gwen Shapira On July 17, 2014, 12:16 a.m., Abraham Elmahrek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23531/ > ----------------------------------------------------------- > > (Updated July 17, 2014, 12:16 a.m.) > > > Review request for Sqoop, Gwen Shapira, Hari Shreedharan, and Jarek Cecho. > > > Bugs: SQOOP-1376 > https://issues.apache.org/jira/browse/SQOOP-1376 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > commit 8327ff3b6432f2d9358991b694945411fff26025 > Author: Abraham Elmahrek <[email protected]> > Date: Wed Jul 9 12:42:17 2014 -0700 > > Sqoop2: From/To: Refactor connector interface > > Various components have been hacked together to compile. > Their working order are to be addressed in follow up Jiras. > This includes, but is not limited to: > Repository, Execution Engine, Submission Engine, Core, > Server, and the shell. > > ConectorBean and JobBean have a simple design. Improvements > are to be addressed in follow up Jiras. > > MConnector now has 2 MJobForms instances. > There is a MJobForms instance for the "from" connector and > a MJobForms instance for the "to" connector. > MConnector still has only one MConnectionForms. > The information needed when extracting using one connector may > be different than the information needed when loading using > the same connector. Example: GenericJdbcConnector can use > a staging table when it is the connector being used to load > data TO. It does not need a staging table when extracting data > FROM. > Since Connections define how Sqoop connects to a database, > there shouldn't be any variation based on FROM/TO. > > MJob objects must now be aware of Connectors that it is > extracting data FROM and sending data TO. It also keeps > track of Connection objects for both Connectors. > > SPI changed to allow FROM/TO. Initializers, laoders, etc. > were not changed, just the classes that extend > CallbackBase. > > :100644 100644 05ea6d6... d1f7a1d... M > client/src/main/java/org/apache/sqoop/client/SqoopClient.java > :100644 100644 cbe049a... f6b2de4... M > common/src/main/java/org/apache/sqoop/json/ConnectorBean.java > :100644 100644 eb79f98... e234908... M > common/src/main/java/org/apache/sqoop/json/FrameworkBean.java > :100644 100644 1555bd5... 58c53d3... M > common/src/main/java/org/apache/sqoop/json/JobBean.java > :000000 100644 0000000... c9a1c42... A > common/src/main/java/org/apache/sqoop/json/JobValidationBean.java > :100644 100644 43fad27... 117ecdc... M > common/src/main/java/org/apache/sqoop/model/MConnector.java > :100644 100644 c742459... 580db9c... M > common/src/main/java/org/apache/sqoop/model/MFramework.java > :100644 100644 849168d... 155f246... M > common/src/main/java/org/apache/sqoop/model/MJob.java > :100644 100644 f697023... 8505483... M > common/src/main/java/org/apache/sqoop/model/MJobForms.java > :100644 100644 e0da80f... ab33568... M > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnector.java > :100644 100644 0c5f6e1... 3e2f89a... M > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcValidator.java > :100644 100644 17215f0... 346b625... M > connector/connector-mysql-jdbc/src/main/java/org/apache/sqoop/connector/mysqljdbc/MySqlJdbcConnector.java > :100644 100644 b80de7f... 9cb6244... M > core/src/main/java/org/apache/sqoop/connector/ConnectorHandler.java > :100644 100644 505121c... fe21a3f... M > core/src/main/java/org/apache/sqoop/framework/FrameworkManager.java > :100644 100644 f5f6a36... 0bd4947... M > core/src/main/java/org/apache/sqoop/framework/FrameworkValidator.java > :100644 100644 e052584... b5bd3a2... M > core/src/main/java/org/apache/sqoop/framework/JobManager.java > :100644 100644 53d0039... 9e8c07c... M > core/src/main/java/org/apache/sqoop/framework/SubmissionRequest.java > :000000 100644 0000000... 7c653bf... A > core/src/main/java/org/apache/sqoop/framework/configuration/JobConfiguration.java > :100644 100644 ecf5004... 95a8bd5... M > core/src/main/java/org/apache/sqoop/repository/Repository.java > :100644 100644 5c0a027... 6352bb5... M > execution/mapreduce/src/main/java/org/apache/sqoop/execution/mapreduce/MapreduceExecutionEngine.java > :100644 100644 bd11323... 96ecf32... M > execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/ConfigurationUtils.java > :100644 100644 e1a95a7... 0edcb7a... M > execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopDestroyerExecutor.java > :100644 100644 6891258... 516c80a... M > execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopInputFormat.java > :100644 100644 92de37e... 37f416d... M > execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopMapper.java > :100644 100644 7dedee9... b5adace... M > execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopOutputFormatLoadExecutor.java > :100644 100644 5bce3a9... f002db6... M > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java > :100644 100644 fcbb475... 1a77360... M > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaConstants.java > :100644 100644 7042a53... fa16360... M > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java > :100644 100644 362ba79... a9394b6... M > server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java > :100644 100644 9cce17b... 99edb6b... M > shell/src/main/java/org/apache/sqoop/shell/CloneConnectionFunction.java > :100644 100644 f80552c... 11878f1... M > shell/src/main/java/org/apache/sqoop/shell/CloneJobFunction.java > :100644 100644 598adbc... e063f9a... M > shell/src/main/java/org/apache/sqoop/shell/CreateJobFunction.java > :100644 100644 54d8e9a... 51e189f... M > shell/src/main/java/org/apache/sqoop/shell/DeleteConnectionFunction.java > :100644 100644 6f07c32... ee1c8c4... M > shell/src/main/java/org/apache/sqoop/shell/DisableConnectionFunction.java > :100644 100644 9256712... 71295fa... M > shell/src/main/java/org/apache/sqoop/shell/EnableConnectionFunction.java > :100644 100644 6e5c9b5... dfaa90e... M > shell/src/main/java/org/apache/sqoop/shell/ShowConnectionFunction.java > :100644 100644 9a5386c... b69b726... M > shell/src/main/java/org/apache/sqoop/shell/ShowJobFunction.java > :100644 100644 8fa1bda... 791cc03... M > shell/src/main/java/org/apache/sqoop/shell/UpdateConnectionFunction.java > :100644 100644 b060bb4... 43baf28... M > shell/src/main/java/org/apache/sqoop/shell/UpdateJobFunction.java > :100644 100644 475f41c... 5765260... M > shell/src/main/java/org/apache/sqoop/shell/core/Constants.java > :100644 100644 56e0b4e... c2f9e22... M > shell/src/main/java/org/apache/sqoop/shell/utils/FormDisplayer.java > :100644 100644 c491ae5... e4b63ee... M > shell/src/main/java/org/apache/sqoop/shell/utils/FormFiller.java > :100644 100644 aa118e1... 7bc6154... M > shell/src/main/java/org/apache/sqoop/shell/utils/JobDynamicFormOptions.java > :100644 100644 2becc56... 7d92ef6... M > spi/src/main/java/org/apache/sqoop/connector/spi/SqoopConnector.java > :100644 000000 cdaa623... 0000000... D > spi/src/main/java/org/apache/sqoop/job/etl/Exporter.java > :000000 100644 0000000... 9b8d76f... A > spi/src/main/java/org/apache/sqoop/job/etl/From.java > :100644 000000 d4c9e70... 0000000... D > spi/src/main/java/org/apache/sqoop/job/etl/Importer.java > :000000 100644 0000000... a791945... A > spi/src/main/java/org/apache/sqoop/job/etl/To.java > :100644 100644 cf0b4aa... 9b791f8... M > spi/src/main/java/org/apache/sqoop/validation/Validator.java > :100644 100644 6fc485b... 54071f2... M > submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java > > > Diffs > ----- > > client/src/main/java/org/apache/sqoop/client/SqoopClient.java 05ea6d6 > common/src/main/java/org/apache/sqoop/json/ConnectorBean.java cbe049a > common/src/main/java/org/apache/sqoop/json/FrameworkBean.java eb79f98 > common/src/main/java/org/apache/sqoop/json/JobBean.java 1555bd5 > common/src/main/java/org/apache/sqoop/json/JobValidationBean.java > PRE-CREATION > common/src/main/java/org/apache/sqoop/model/MConnector.java 43fad27 > common/src/main/java/org/apache/sqoop/model/MFramework.java c742459 > common/src/main/java/org/apache/sqoop/model/MJob.java 849168d > common/src/main/java/org/apache/sqoop/model/MJobForms.java f697023 > > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnector.java > e0da80f > > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcValidator.java > 0c5f6e1 > > connector/connector-mysql-jdbc/src/main/java/org/apache/sqoop/connector/mysqljdbc/MySqlJdbcConnector.java > 17215f0 > core/src/main/java/org/apache/sqoop/connector/ConnectorHandler.java b80de7f > core/src/main/java/org/apache/sqoop/framework/FrameworkManager.java 505121c > core/src/main/java/org/apache/sqoop/framework/FrameworkValidator.java > f5f6a36 > core/src/main/java/org/apache/sqoop/framework/JobManager.java e052584 > core/src/main/java/org/apache/sqoop/framework/SubmissionRequest.java > 53d0039 > > core/src/main/java/org/apache/sqoop/framework/configuration/JobConfiguration.java > PRE-CREATION > core/src/main/java/org/apache/sqoop/repository/Repository.java ecf5004 > > execution/mapreduce/src/main/java/org/apache/sqoop/execution/mapreduce/MapreduceExecutionEngine.java > 5c0a027 > > execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/ConfigurationUtils.java > bd11323 > > execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopDestroyerExecutor.java > e1a95a7 > > execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopInputFormat.java > 6891258 > execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopMapper.java > 92de37e > > execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopOutputFormatLoadExecutor.java > 7dedee9 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java > 5bce3a9 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaConstants.java > fcbb475 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java > 7042a53 > server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java > 362ba79 > shell/src/main/java/org/apache/sqoop/shell/CloneConnectionFunction.java > 9cce17b > shell/src/main/java/org/apache/sqoop/shell/CloneJobFunction.java f80552c > shell/src/main/java/org/apache/sqoop/shell/CreateJobFunction.java 598adbc > shell/src/main/java/org/apache/sqoop/shell/DeleteConnectionFunction.java > 54d8e9a > shell/src/main/java/org/apache/sqoop/shell/DisableConnectionFunction.java > 6f07c32 > shell/src/main/java/org/apache/sqoop/shell/EnableConnectionFunction.java > 9256712 > shell/src/main/java/org/apache/sqoop/shell/ShowConnectionFunction.java > 6e5c9b5 > shell/src/main/java/org/apache/sqoop/shell/ShowJobFunction.java 9a5386c > shell/src/main/java/org/apache/sqoop/shell/UpdateConnectionFunction.java > 8fa1bda > shell/src/main/java/org/apache/sqoop/shell/UpdateJobFunction.java b060bb4 > shell/src/main/java/org/apache/sqoop/shell/core/Constants.java 475f41c > shell/src/main/java/org/apache/sqoop/shell/utils/FormDisplayer.java 56e0b4e > shell/src/main/java/org/apache/sqoop/shell/utils/FormFiller.java c491ae5 > shell/src/main/java/org/apache/sqoop/shell/utils/JobDynamicFormOptions.java > aa118e1 > spi/src/main/java/org/apache/sqoop/connector/spi/SqoopConnector.java > 2becc56 > spi/src/main/java/org/apache/sqoop/job/etl/Exporter.java cdaa623 > spi/src/main/java/org/apache/sqoop/job/etl/From.java PRE-CREATION > spi/src/main/java/org/apache/sqoop/job/etl/Importer.java d4c9e70 > spi/src/main/java/org/apache/sqoop/job/etl/To.java PRE-CREATION > spi/src/main/java/org/apache/sqoop/validation/Validator.java cf0b4aa > > submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java > 6fc485b > > Diff: https://reviews.apache.org/r/23531/diff/ > > > Testing > ------- > > N/A > > This patch should break just about every thing. What needs to be verified is > that the interface is good and the models are fine. > > > Thanks, > > Abraham Elmahrek > >
