----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40522/#review107394 -----------------------------------------------------------
Overall looks good to me, I have several nits: repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java (lines 379 - 380) <https://reviews.apache.org/r/40522/#comment166486> Can't we just change the query to join with the connector table and search by the connector name directly? Seems unnecessary to construct the whole MConnector structure here. repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java (line 1541) <https://reviews.apache.org/r/40522/#comment166488> Can we change the queries to resolve the connector name (additinal join)? repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java (line 1585) <https://reviews.apache.org/r/40522/#comment166487> Can we change the queries to resolve the connector name (additinal join)? test/src/main/java/org/apache/sqoop/test/testcases/ConnectorTestCase.java (lines 309 - 361) <https://reviews.apache.org/r/40522/#comment166490> test/src/test/java/org/apache/sqoop/integration/connector/hdfs/AppendModeTest.java (lines 49 - 51) <https://reviews.apache.org/r/40522/#comment166491> Could you describe why do we need to do the manual clear here? We're creating new MJob, so the values should be empty by default. Are we just missing calling .clone(false) somewhere? Jarcec - Jarek Cecho On Nov. 20, 2015, 6 a.m., Colin Ma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40522/ > ----------------------------------------------------------- > > (Updated Nov. 20, 2015, 6 a.m.) > > > Review request for Sqoop. > > > Repository: sqoop-sqoop2 > > > Description > ------- > > Currently connector id is used in MLink as a foreigner key, the connector id > will be removed from public API, this should be updated as connector name. > > > Diffs > ----- > > client/src/main/java/org/apache/sqoop/client/SqoopClient.java 3d3425d > > client/src/main/java/org/apache/sqoop/client/request/ConnectorResourceRequest.java > f23c6e4 > > client/src/main/java/org/apache/sqoop/client/request/SqoopResourceRequests.java > 0987703 > client/src/test/java/org/apache/sqoop/client/TestSqoopClient.java c962358 > common/src/main/java/org/apache/sqoop/json/ConnectorBean.java 89ae20c > common/src/main/java/org/apache/sqoop/json/ConnectorsBean.java 9762ffa > common/src/main/java/org/apache/sqoop/json/LinkBean.java c9a8ab3 > common/src/main/java/org/apache/sqoop/model/MLink.java 51a4008 > common/src/test/java/org/apache/sqoop/json/TestConnectorBean.java a978d62 > common/src/test/java/org/apache/sqoop/json/TestConnectorsBean.java 15377c9 > common/src/test/java/org/apache/sqoop/json/util/BeanTestUtil.java 51ab0e5 > common/src/test/java/org/apache/sqoop/model/TestMAccountableEntity.java > 5bf2465 > common/src/test/java/org/apache/sqoop/model/TestMLink.java b5df18a > core/src/main/java/org/apache/sqoop/connector/ConnectorManager.java db626a1 > core/src/main/java/org/apache/sqoop/driver/JobManager.java 90ee541 > core/src/test/java/org/apache/sqoop/driver/TestJobManager.java f137eef > core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java > 00b0511 > > repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java > d1ee320 > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestInputTypes.java > 193ee5f > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java > 92d1fae > > repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/MySqlTestCase.java > 232ef4c > > repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/PostgresqlTestCase.java > b8b0f52 > server/src/main/java/org/apache/sqoop/handler/ConnectorRequestHandler.java > 52abe72 > server/src/main/java/org/apache/sqoop/handler/LinkRequestHandler.java > 9e75258 > shell/src/main/java/org/apache/sqoop/shell/CloneJobFunction.java 15cfad7 > shell/src/main/java/org/apache/sqoop/shell/CloneLinkFunction.java a679be8 > shell/src/main/java/org/apache/sqoop/shell/CreateJobFunction.java 79caa0d > shell/src/main/java/org/apache/sqoop/shell/CreateLinkFunction.java 289c3c3 > shell/src/main/java/org/apache/sqoop/shell/ShowConnectorFunction.java > 6efb51c > shell/src/main/java/org/apache/sqoop/shell/ShowJobFunction.java 21873cc > shell/src/main/java/org/apache/sqoop/shell/ShowLinkFunction.java 04dd228 > shell/src/main/java/org/apache/sqoop/shell/UpdateJobFunction.java 49cfd0b > shell/src/main/java/org/apache/sqoop/shell/UpdateLinkFunction.java 1bb7cd5 > shell/src/test/java/org/apache/sqoop/shell/TestCloneCommand.java eed27d7 > shell/src/test/java/org/apache/sqoop/shell/TestCreateCommand.java 48f646b > shell/src/test/java/org/apache/sqoop/shell/TestShowCommand.java 4272386 > shell/src/test/java/org/apache/sqoop/shell/TestUpdateCommand.java 9b3e87a > test/src/main/java/org/apache/sqoop/test/testcases/ConnectorTestCase.java > c843448 > > test/src/test/java/org/apache/sqoop/integration/connector/hdfs/AppendModeTest.java > 8c65898 > > test/src/test/java/org/apache/sqoop/integration/connector/hdfs/FromHDFSToHDFSTest.java > c39c8d6 > > test/src/test/java/org/apache/sqoop/integration/connector/hdfs/HdfsIncrementalReadTest.java > e6f6e0d > > test/src/test/java/org/apache/sqoop/integration/connector/hdfs/InformalJobNameExecuteTest.java > 411b07e > > test/src/test/java/org/apache/sqoop/integration/connector/hdfs/OutputDirectoryTest.java > 1790f96 > test/src/test/java/org/apache/sqoop/integration/connector/hdfs/S3Test.java > 7fec310 > > test/src/test/java/org/apache/sqoop/integration/connector/hive/FromRDBMSToKiteHiveTest.java > 0e46bf3 > > test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/AllTypesTest.java > 5053b56 > > test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/FromHDFSToRDBMSTest.java > 25cdb68 > > test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/FromRDBMSToHDFSTest.java > 686572a > > test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/IncrementalReadTest.java > 38ebb74 > > test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/PartitionerTest.java > 72728fe > > test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/TableStagedRDBMSTest.java > 68dc65e > > test/src/test/java/org/apache/sqoop/integration/connector/kafka/FromHDFSToKafkaTest.java > aa062fb > > test/src/test/java/org/apache/sqoop/integration/connector/kafka/FromRDBMSToKafkaTest.java > 6e78a13 > > test/src/test/java/org/apache/sqoop/integration/connector/kite/FromRDBMSToKiteTest.java > 7b2aced > > test/src/test/java/org/apache/sqoop/integration/server/InformalObjectNameTest.java > 920679f > > test/src/test/java/org/apache/sqoop/integration/server/ShowJobInOrderTest.java > cbf1e90 > tools/src/main/java/org/apache/sqoop/tools/tool/RepositoryLoadTool.java > 75d2182 > > Diff: https://reviews.apache.org/r/40522/diff/ > > > Testing > ------- > > > Thanks, > > Colin Ma > >
