----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40522/#review108012 -----------------------------------------------------------
Thanks for pushing this one forward: client/src/main/java/org/apache/sqoop/client/SqoopClient.java (lines 400 - 401) <https://reviews.apache.org/r/40522/#comment167384> To follow up the discussion we had on previous patch - I don't think that we should return reference to the internal structures here as you found out that it means that we can't call more then one createJob(). I think that we should return copy of the structures instead, e.g.: connectorForFromLink.getFromConfig().clone(false) What do you think? repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java (line 1520) <https://reviews.apache.org/r/40522/#comment167387> Nit: Unused variable? repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java (line 1568) <https://reviews.apache.org/r/40522/#comment167388> Nit: Unused variable? test/src/main/java/org/apache/sqoop/test/testcases/JettyTestCase.java (lines 106 - 109) <https://reviews.apache.org/r/40522/#comment167381> I have a concern that this is hiding the real problem - that we're returning mutable objects that are referencing cache in SqoopClient. Let's get it fixed there rather then doing this. Jarcec - Jarek Cecho On Nov. 23, 2015, 4:40 a.m., Colin Ma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40522/ > ----------------------------------------------------------- > > (Updated Nov. 23, 2015, 4:40 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/JettyTestCase.java > bd4ba6a > test/src/test/java/org/apache/sqoop/integration/connector/hdfs/S3Test.java > 7fec310 > > 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 > >
