You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Dian Fu <di...@gmail.com> on 2015/09/16 09:33:28 UTC
Review Request 38419: Sqoop2: Use object name instead of object id
for job, link and connector in Sqoop 2 server
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38419/
-----------------------------------------------------------
Review request for Sqoop.
Bugs: SQOOP-2573
https://issues.apache.org/jira/browse/SQOOP-2573
Repository: sqoop-sqoop2
Description
-------
Use object name instead of object id for job, link and connector in Sqoop 2 server.
Diffs
-----
core/src/main/java/org/apache/sqoop/connector/ConnectorManager.java 5f269aa
core/src/main/java/org/apache/sqoop/driver/JobManager.java 87c6c74
core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java d7aa8ef
core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 5cbeda8
core/src/main/java/org/apache/sqoop/repository/Repository.java 10af697
core/src/main/java/org/apache/sqoop/repository/RepositoryError.java f684e85
core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java 4999159
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java c2ba6e3
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java 560fd58
repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java 6c36f02
repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java 4be3641
repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestSubmissionHandling.java 405c0b8
repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestJobHandling.java 0614d7a
repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestLinkHandling.java cceef09
repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestSubmissionHandling.java e2a3011
repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestJobHandling.java 2ad43dd
repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestLinkHandling.java 92bb15b
repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestSubmissionHandling.java c97b4f8
server/src/main/java/org/apache/sqoop/handler/ConnectorRequestHandler.java 60b124c
server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java cb9485f
server/src/main/java/org/apache/sqoop/handler/LinkRequestHandler.java 3041646
server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java 4b99239
Diff: https://reviews.apache.org/r/38419/diff/
Testing
-------
Thanks,
Dian Fu
Re: Review Request 38419: Sqoop2: Use object name instead of object
id for job, link and connector in Sqoop 2 server
Posted by Dian Fu <di...@gmail.com>.
> On Sept. 16, 2015, 10:06 p.m., Jarek Cecho wrote:
> > core/src/main/java/org/apache/sqoop/driver/JobManager.java, line 280
> > <https://reviews.apache.org/r/38419/diff/1/?file=1075168#file1075168line280>
> >
> > Shouldn't we drop the jobId parameter and refactore the method accordingly?
> >
> > (I have the same question for other methods in this file)
Thanks Jarcec for the review. The return type of start(), stop() and status() is MSubmission which has an element jobId. As MSubmission will be returned to the client side and to be backward compatible, I kept the jobId parameter here. But it definitely would be better if we can drop this parameter. Any advice?
- Dian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38419/#review99315
-----------------------------------------------------------
On Sept. 16, 2015, 7:33 a.m., Dian Fu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38419/
> -----------------------------------------------------------
>
> (Updated Sept. 16, 2015, 7:33 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-2573
> https://issues.apache.org/jira/browse/SQOOP-2573
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> Use object name instead of object id for job, link and connector in Sqoop 2 server.
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/sqoop/connector/ConnectorManager.java 5f269aa
> core/src/main/java/org/apache/sqoop/driver/JobManager.java 87c6c74
> core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java d7aa8ef
> core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 5cbeda8
> core/src/main/java/org/apache/sqoop/repository/Repository.java 10af697
> core/src/main/java/org/apache/sqoop/repository/RepositoryError.java f684e85
> core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java 4999159
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java c2ba6e3
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java 560fd58
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java 6c36f02
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java 4be3641
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestSubmissionHandling.java 405c0b8
> repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestJobHandling.java 0614d7a
> repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestLinkHandling.java cceef09
> repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestSubmissionHandling.java e2a3011
> repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestJobHandling.java 2ad43dd
> repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestLinkHandling.java 92bb15b
> repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestSubmissionHandling.java c97b4f8
> server/src/main/java/org/apache/sqoop/handler/ConnectorRequestHandler.java 60b124c
> server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java cb9485f
> server/src/main/java/org/apache/sqoop/handler/LinkRequestHandler.java 3041646
> server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java 4b99239
>
> Diff: https://reviews.apache.org/r/38419/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dian Fu
>
>
Re: Review Request 38419: Sqoop2: Use object name instead of object
id for job, link and connector in Sqoop 2 server
Posted by Jarek Cecho <ja...@apache.org>.
> On Sept. 16, 2015, 10:06 p.m., Jarek Cecho wrote:
> > core/src/main/java/org/apache/sqoop/driver/JobManager.java, line 280
> > <https://reviews.apache.org/r/38419/diff/1/?file=1075168#file1075168line280>
> >
> > Shouldn't we drop the jobId parameter and refactore the method accordingly?
> >
> > (I have the same question for other methods in this file)
>
> Dian Fu wrote:
> Thanks Jarcec for the review. The return type of start(), stop() and status() is MSubmission which has an element jobId. As MSubmission will be returned to the client side and to be backward compatible, I kept the jobId parameter here. But it definitely would be better if we can drop this parameter. Any advice?
I was originally thinking that the client is giving us only the "jobName", so we have to resolve the id somewhere - it seems that we're doing that in Handler code (HandlerUtils.getJobIdFromIdentifier). Let's perhaps punt this point to a seperate JIRA? The patch is "huge" already without introducing yet another movement of code.
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38419/#review99315
-----------------------------------------------------------
On Sept. 16, 2015, 7:33 a.m., Dian Fu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38419/
> -----------------------------------------------------------
>
> (Updated Sept. 16, 2015, 7:33 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-2573
> https://issues.apache.org/jira/browse/SQOOP-2573
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> Use object name instead of object id for job, link and connector in Sqoop 2 server.
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/sqoop/connector/ConnectorManager.java 5f269aa
> core/src/main/java/org/apache/sqoop/driver/JobManager.java 87c6c74
> core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java d7aa8ef
> core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 5cbeda8
> core/src/main/java/org/apache/sqoop/repository/Repository.java 10af697
> core/src/main/java/org/apache/sqoop/repository/RepositoryError.java f684e85
> core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java 4999159
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java c2ba6e3
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java 560fd58
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java 6c36f02
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java 4be3641
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestSubmissionHandling.java 405c0b8
> repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestJobHandling.java 0614d7a
> repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestLinkHandling.java cceef09
> repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestSubmissionHandling.java e2a3011
> repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestJobHandling.java 2ad43dd
> repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestLinkHandling.java 92bb15b
> repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestSubmissionHandling.java c97b4f8
> server/src/main/java/org/apache/sqoop/handler/ConnectorRequestHandler.java 60b124c
> server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java cb9485f
> server/src/main/java/org/apache/sqoop/handler/LinkRequestHandler.java 3041646
> server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java 4b99239
>
> Diff: https://reviews.apache.org/r/38419/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dian Fu
>
>
Re: Review Request 38419: Sqoop2: Use object name instead of object
id for job, link and connector in Sqoop 2 server
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38419/#review99315
-----------------------------------------------------------
I have one question:
core/src/main/java/org/apache/sqoop/driver/JobManager.java (line 280)
<https://reviews.apache.org/r/38419/#comment156220>
Shouldn't we drop the jobId parameter and refactore the method accordingly?
(I have the same question for other methods in this file)
Jarcec
- Jarek Cecho
On Sept. 16, 2015, 7:33 a.m., Dian Fu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38419/
> -----------------------------------------------------------
>
> (Updated Sept. 16, 2015, 7:33 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-2573
> https://issues.apache.org/jira/browse/SQOOP-2573
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> Use object name instead of object id for job, link and connector in Sqoop 2 server.
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/sqoop/connector/ConnectorManager.java 5f269aa
> core/src/main/java/org/apache/sqoop/driver/JobManager.java 87c6c74
> core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java d7aa8ef
> core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 5cbeda8
> core/src/main/java/org/apache/sqoop/repository/Repository.java 10af697
> core/src/main/java/org/apache/sqoop/repository/RepositoryError.java f684e85
> core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java 4999159
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java c2ba6e3
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java 560fd58
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java 6c36f02
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java 4be3641
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestSubmissionHandling.java 405c0b8
> repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestJobHandling.java 0614d7a
> repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestLinkHandling.java cceef09
> repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestSubmissionHandling.java e2a3011
> repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestJobHandling.java 2ad43dd
> repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestLinkHandling.java 92bb15b
> repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestSubmissionHandling.java c97b4f8
> server/src/main/java/org/apache/sqoop/handler/ConnectorRequestHandler.java 60b124c
> server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java cb9485f
> server/src/main/java/org/apache/sqoop/handler/LinkRequestHandler.java 3041646
> server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java 4b99239
>
> Diff: https://reviews.apache.org/r/38419/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dian Fu
>
>
Re: Review Request 38419: Sqoop2: Use object name instead of object
id for job, link and connector in Sqoop 2 server
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38419/#review99384
-----------------------------------------------------------
Ship it!
Ship It!
- Jarek Cecho
On Sept. 16, 2015, 7:33 a.m., Dian Fu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38419/
> -----------------------------------------------------------
>
> (Updated Sept. 16, 2015, 7:33 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-2573
> https://issues.apache.org/jira/browse/SQOOP-2573
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> Use object name instead of object id for job, link and connector in Sqoop 2 server.
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/sqoop/connector/ConnectorManager.java 5f269aa
> core/src/main/java/org/apache/sqoop/driver/JobManager.java 87c6c74
> core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java d7aa8ef
> core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 5cbeda8
> core/src/main/java/org/apache/sqoop/repository/Repository.java 10af697
> core/src/main/java/org/apache/sqoop/repository/RepositoryError.java f684e85
> core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java 4999159
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java c2ba6e3
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java 560fd58
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java 6c36f02
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java 4be3641
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestSubmissionHandling.java 405c0b8
> repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestJobHandling.java 0614d7a
> repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestLinkHandling.java cceef09
> repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestSubmissionHandling.java e2a3011
> repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestJobHandling.java 2ad43dd
> repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestLinkHandling.java 92bb15b
> repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestSubmissionHandling.java c97b4f8
> server/src/main/java/org/apache/sqoop/handler/ConnectorRequestHandler.java 60b124c
> server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java cb9485f
> server/src/main/java/org/apache/sqoop/handler/LinkRequestHandler.java 3041646
> server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java 4b99239
>
> Diff: https://reviews.apache.org/r/38419/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dian Fu
>
>