> On Nov. 20, 2015, 7:10 p.m., Jarek Cecho wrote: > > repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java, > > lines 379-380 > > <https://reviews.apache.org/r/40522/diff/2/?file=1133763#file1133763line379> > > > > 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. > > Colin Ma wrote: > This is an INSERT statement, and we have to get the connector id by the > SQL. > I agree that whole MConnector is unnecessary here. To get connectorId > only, there should be a new SQL like: select connectorId from connectorTable > where connectorName = XXXX. > What do you think? reuse the current heavy function or create a new > simple SQL and function? > If we need create a new SQL, how about create a new JIRA for it?
Good point Colin, let's introduce the new SQL for retrieving connector ID for connector name and let's do it as a part of separate JIRA. > On Nov. 20, 2015, 7:10 p.m., Jarek Cecho wrote: > > test/src/test/java/org/apache/sqoop/integration/connector/hdfs/AppendModeTest.java, > > lines 49-51 > > <https://reviews.apache.org/r/40522/diff/2/?file=1133784#file1133784line49> > > > > 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? > > Colin Ma wrote: > Check the again, currently, we get the MFromConfig, MToConfig from cache > in SqoopClient. The SqoopClient is shared in all test cases under one suite. > Some test cases get the MFromConfig/MToConfig, change it manual and save > it back to cache again, for the next test case, the MFromConfig/MToConfig > will be invalid. > Clear the cache after every test case can fix it, will drop the current > implementation for this problem. > Thanks for reminder this. I've followed up on this one on the later patch version. - Jarek ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40522/#review107394 ----------------------------------------------------------- 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 > >
