sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Veena Basavaraj" <vbasava...@cloudera.com>
Subject Re: Review Request 26941: SQOOP-1557: SQ_CONFIGURABLE ( for entities who own configs)
Date Tue, 21 Oct 2014 21:39:05 GMT


> On Oct. 21, 2014, 1:47 p.m., Jarek Cecho wrote:
> > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java,
line 2317
> > <https://reviews.apache.org/r/26941/diff/3/?file=727768#file727768line2317>
> >
> >     Can we keep the comment here so that it's obvious what we're doing here?

that statement is commented out as well,


> On Oct. 21, 2014, 1:47 p.m., Jarek Cecho wrote:
> > core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java, line 224
> > <https://reviews.apache.org/r/26941/diff/3/?file=727762#file727762line224>
> >
> >     I'm thinking if it would be claner to call here mDriver.getUniqueName() instead
of going to the Driver class directly?
> >     
> >     It's more a nitpick at this point, but this methods assumes that the mDriver
will be derived from Driver. Which right now will definitely be the case, but I'm wondering
whether we want to have such assumption here.

I had it MDriver and somehow I could not use it one class because of cyclic import dependency,
so I had to move it to Driver.

I can change it back


- Veena


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


On Oct. 21, 2014, 12:19 p.m., Veena Basavaraj wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26941/
> -----------------------------------------------------------
> 
> (Updated Oct. 21, 2014, 12:19 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> see JIRA for details
> 
> there is whitespace, that will be addressed once the reviews for the functionality
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/sqoop/model/MConnector.java 174d0b9 
>   common/src/main/java/org/apache/sqoop/model/MDriver.java 4241a31 
>   core/src/main/java/org/apache/sqoop/driver/Driver.java 46a16ac 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 476830d 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 4c5229f 
>   core/src/main/java/org/apache/sqoop/repository/Repository.java 8f78052 
>   core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java ff9e0c3 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoConstants.java
40dcc49 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java
3e4a4a9 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
aa58850 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaConstants.java
59773e1 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java
44ec2e3 
>   repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/DerbyTestCase.java
366e4ee 
>   repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestConnectorHandling.java
68a173b 
>   repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestDriverHandling.java
bbf721f 
>   repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java
a15bda9 
>   tools/src/main/java/org/apache/sqoop/tools/tool/RepositoryLoadTool.java 8cf9cf1 
> 
> Diff: https://reviews.apache.org/r/26941/diff/
> 
> 
> Testing
> -------
> 
> yes
> 
> 
> Thanks,
> 
> Veena Basavaraj
> 
>


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