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 22034: SQOOP 1331 - Add method "findConnectors" to repository API
Date Fri, 30 May 2014 01:30:18 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22034/#review44322
-----------------------------------------------------------


Couple of other comments:


repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
<https://reviews.apache.org/r/22034/#comment78697>

    I would put the debug message as the first statement in the method rather then at the
end.



repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
<https://reviews.apache.org/r/22034/#comment78696>

    Removing the finally section do not seem to be completely accurate here, we still want
to close the baseConnectorFetchStmt, right?



repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestConnectorHandling.java
<https://reviews.apache.org/r/22034/#comment78698>

    Nit: Please use the one line comment that starts with "//"



repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestConnectorHandling.java
<https://reviews.apache.org/r/22034/#comment78699>

    Super nit: please put space between "//" and "Retrieve"


- Jarek Cecho


On May 29, 2014, 7:53 p.m., Gwen Shapira wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22034/
> -----------------------------------------------------------
> 
> (Updated May 29, 2014, 7:53 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> Adding a method to get all connectors to the repository API.
> 
> Note that while this patch adds a new method, it also modifies the existing findConnector(String
shortName) to avoid duplicating code.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 7768b13 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 9299484 
>   core/src/main/java/org/apache/sqoop/repository/Repository.java 4c7b866 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java
e4b30f9 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
4f002bb 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java
c470211 
>   repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/DerbyTestCase.java
a1ad40d 
>   repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestConnectorHandling.java
cc3fe60 
> 
> Diff: https://reviews.apache.org/r/22034/diff/
> 
> 
> Testing
> -------
> 
> Added unit-test for the new method. Did basic repository functionality testing on a cluster.
> 
> 
> Thanks,
> 
> Gwen Shapira
> 
>


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