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/08/26 11:30:25 UTC
Review Request 37793: Sqoop2: Add utility methods to get object name
from object id
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37793/
-----------------------------------------------------------
Review request for Sqoop.
Bugs: SQOOP-2530
https://issues.apache.org/jira/browse/SQOOP-2530
Repository: sqoop-sqoop2
Description
-------
We need add some utility methods to translate connectorId/linkId/jobId to the corresponding connectorName/linkName/jobName.
Diffs
-----
common/src/main/java/org/apache/sqoop/error/code/CommonRepositoryError.java 9f4a0f8
core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 9a4853b
core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java f690887
core/src/main/java/org/apache/sqoop/repository/Repository.java 610876c
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java af9324f
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java 4f9e547
server/src/main/java/org/apache/sqoop/handler/HandlerUtils.java 5fcde52
server/src/main/java/org/apache/sqoop/server/common/ServerError.java c68ab57
Diff: https://reviews.apache.org/r/37793/diff/
Testing
-------
Thanks,
Dian Fu
Re: Review Request 37793: Sqoop2: Add utility methods to get object
name from object id
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37793/#review96690
-----------------------------------------------------------
Ship it!
Ship It!
- Jarek Cecho
On Aug. 27, 2015, 6:42 a.m., Dian Fu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37793/
> -----------------------------------------------------------
>
> (Updated Aug. 27, 2015, 6:42 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-2530
> https://issues.apache.org/jira/browse/SQOOP-2530
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> We need add some utility methods to translate connectorId/linkId/jobId to the corresponding connectorName/linkName/jobName.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/error/code/CommonRepositoryError.java 9f4a0f8
> core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 9a4853b
> core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java f690887
> core/src/main/java/org/apache/sqoop/repository/Repository.java 610876c
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java af9324f
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java 4f9e547
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestConnectorHandling.java 8864d5d
> repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestConnectorHandling.java 8e1b3d1
> repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestConnectorHandling.java 2dcab48
> server/src/main/java/org/apache/sqoop/handler/HandlerUtils.java 5fcde52
> server/src/main/java/org/apache/sqoop/server/common/ServerError.java c68ab57
>
> Diff: https://reviews.apache.org/r/37793/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dian Fu
>
>
Re: Review Request 37793: Sqoop2: Add utility methods to get object
name from object id
Posted by Dian Fu <di...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37793/
-----------------------------------------------------------
(Updated Aug. 27, 2015, 6:42 a.m.)
Review request for Sqoop.
Bugs: SQOOP-2530
https://issues.apache.org/jira/browse/SQOOP-2530
Repository: sqoop-sqoop2
Description
-------
We need add some utility methods to translate connectorId/linkId/jobId to the corresponding connectorName/linkName/jobName.
Diffs (updated)
-----
common/src/main/java/org/apache/sqoop/error/code/CommonRepositoryError.java 9f4a0f8
core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 9a4853b
core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java f690887
core/src/main/java/org/apache/sqoop/repository/Repository.java 610876c
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java af9324f
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java 4f9e547
repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestConnectorHandling.java 8864d5d
repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestConnectorHandling.java 8e1b3d1
repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestConnectorHandling.java 2dcab48
server/src/main/java/org/apache/sqoop/handler/HandlerUtils.java 5fcde52
server/src/main/java/org/apache/sqoop/server/common/ServerError.java c68ab57
Diff: https://reviews.apache.org/r/37793/diff/
Testing
-------
Thanks,
Dian Fu
Re: Review Request 37793: Sqoop2: Add utility methods to get object
name from object id
Posted by Dian Fu <di...@gmail.com>.
> On Aug. 26, 2015, 4:35 p.m., Jarek Cecho wrote:
> > Thanks for the patch Dian! Couple of high level points:
> >
> > 1) Can we add tests for the new functionality?
Thanks Jarcec for the review. Updated the patch to add new tests and address the code review comments.
- Dian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37793/#review96542
-----------------------------------------------------------
On Aug. 26, 2015, 9:30 a.m., Dian Fu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37793/
> -----------------------------------------------------------
>
> (Updated Aug. 26, 2015, 9:30 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-2530
> https://issues.apache.org/jira/browse/SQOOP-2530
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> We need add some utility methods to translate connectorId/linkId/jobId to the corresponding connectorName/linkName/jobName.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/error/code/CommonRepositoryError.java 9f4a0f8
> core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 9a4853b
> core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java f690887
> core/src/main/java/org/apache/sqoop/repository/Repository.java 610876c
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java af9324f
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java 4f9e547
> server/src/main/java/org/apache/sqoop/handler/HandlerUtils.java 5fcde52
> server/src/main/java/org/apache/sqoop/server/common/ServerError.java c68ab57
>
> Diff: https://reviews.apache.org/r/37793/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dian Fu
>
>
Re: Review Request 37793: Sqoop2: Add utility methods to get object
name from object id
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37793/#review96542
-----------------------------------------------------------
Thanks for the patch Dian! Couple of high level points:
1) Can we add tests for the new functionality?
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java (lines 100 - 126)
<https://reviews.apache.org/r/37793/#comment151989>
It seems to very high extent copy&pasted code from the findConnector(String, Connection) method. I"m wondering if we can do slightly better and somehow consolidate those method? Perhaps by doing findConnectorInternal(String query, Connection) that will run entire code here with small "wrappers" on top that will just supply the "right" query?
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java (lines 40 - 50)
<https://reviews.apache.org/r/37793/#comment151991>
Similarly for the queries - let's perhaps do base "SELECT_FROM_CONFIGURABLE" query without the WHERE clause and use that to define the additional queries? I do not insist on consolidating the queries, so just an idea here.
Jarcec
- Jarek Cecho
On Aug. 26, 2015, 9:30 a.m., Dian Fu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37793/
> -----------------------------------------------------------
>
> (Updated Aug. 26, 2015, 9:30 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-2530
> https://issues.apache.org/jira/browse/SQOOP-2530
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> We need add some utility methods to translate connectorId/linkId/jobId to the corresponding connectorName/linkName/jobName.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/error/code/CommonRepositoryError.java 9f4a0f8
> core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 9a4853b
> core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java f690887
> core/src/main/java/org/apache/sqoop/repository/Repository.java 610876c
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java af9324f
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java 4f9e547
> server/src/main/java/org/apache/sqoop/handler/HandlerUtils.java 5fcde52
> server/src/main/java/org/apache/sqoop/server/common/ServerError.java c68ab57
>
> Diff: https://reviews.apache.org/r/37793/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dian Fu
>
>