sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jarek Cecho" <jar...@apache.org>
Subject Re: Review Request 40522: SQOOP-2690: Sqoop2: Use connector name in MLink.
Date Wed, 25 Nov 2015 16:19:58 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message