You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Veena Basavaraj <vb...@cloudera.com> on 2014/10/20 18:46:04 UTC
Re: Review Request 26941: SQOOP-1557: SQ_CONFIGURABLE ( for entities
who own configs)
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26941/
-----------------------------------------------------------
(Updated Oct. 20, 2014, 9:46 a.m.)
Review request for Sqoop.
Summary (updated)
-----------------
SQOOP-1557: SQ_CONFIGURABLE ( for entities who own configs)
Repository: sqoop-sqoop2
Description
-------
see JIRA for details
Diffs
-----
common/src/main/java/org/apache/sqoop/model/MConfigurableType.java PRE-CREATION
common/src/main/java/org/apache/sqoop/model/MConnector.java 2f42191
common/src/main/java/org/apache/sqoop/model/MDriver.java 685439e
core/src/main/java/org/apache/sqoop/driver/Driver.java f1b45bb
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 c888910
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
Diff: https://reviews.apache.org/r/26941/diff/
Testing
-------
yes
Thanks,
Veena Basavaraj
Re: Review Request 26941: SQOOP-1557: SQ_CONFIGURABLE ( for entities
who own configs)
Posted by Jarek Cecho <ja...@apache.org>.
> On Oct. 20, 2014, 11:06 p.m., Veena Basavaraj wrote:
> > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java, line 194
> > <https://reviews.apache.org/r/26941/diff/1/?file=725835#file725835line194>
> >
> > I have fixed this, but I was curious to know if this is a standard style? or a personal preference.
The reason is to enable better use of "git cherrypick" and "git blame", so that by adding new error exception you don't have to alter unrelated line before.
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26941/#review57441
-----------------------------------------------------------
On Oct. 20, 2014, 6:27 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26941/
> -----------------------------------------------------------
>
> (Updated Oct. 20, 2014, 6:27 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/MConfigurableType.java PRE-CREATION
> common/src/main/java/org/apache/sqoop/model/MConnector.java 2f42191
> common/src/main/java/org/apache/sqoop/model/MDriver.java 685439e
> core/src/main/java/org/apache/sqoop/driver/Driver.java f1b45bb
> 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 c888910
> 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
>
> Diff: https://reviews.apache.org/r/26941/diff/
>
>
> Testing
> -------
>
> yes
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 26941: SQOOP-1557: SQ_CONFIGURABLE ( for entities
who own configs)
Posted by Veena Basavaraj <vb...@cloudera.com>.
> On Oct. 20, 2014, 4:06 p.m., Veena Basavaraj wrote:
> > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java, line 194
> > <https://reviews.apache.org/r/26941/diff/1/?file=725835#file725835line194>
> >
> > I have fixed this, but I was curious to know if this is a standard style? or a personal preference.
>
> Jarek Cecho wrote:
> The reason is to enable better use of "git cherrypick" and "git blame", so that by adding new error exception you don't have to alter unrelated line before.
ah. ! light bulb.
- Veena
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26941/#review57441
-----------------------------------------------------------
On Oct. 20, 2014, 11:27 a.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26941/
> -----------------------------------------------------------
>
> (Updated Oct. 20, 2014, 11:27 a.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/MConfigurableType.java PRE-CREATION
> common/src/main/java/org/apache/sqoop/model/MConnector.java 2f42191
> common/src/main/java/org/apache/sqoop/model/MDriver.java 685439e
> core/src/main/java/org/apache/sqoop/driver/Driver.java f1b45bb
> 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 c888910
> 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
>
> Diff: https://reviews.apache.org/r/26941/diff/
>
>
> Testing
> -------
>
> yes
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 26941: SQOOP-1557: SQ_CONFIGURABLE ( for entities
who own configs)
Posted by Veena Basavaraj <vb...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26941/#review57441
-----------------------------------------------------------
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java
<https://reviews.apache.org/r/26941/#comment98140>
I have fixed this, but I was curious to know if this is a standard style? or a personal preference.
- Veena Basavaraj
On Oct. 20, 2014, 11:27 a.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26941/
> -----------------------------------------------------------
>
> (Updated Oct. 20, 2014, 11:27 a.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/MConfigurableType.java PRE-CREATION
> common/src/main/java/org/apache/sqoop/model/MConnector.java 2f42191
> common/src/main/java/org/apache/sqoop/model/MDriver.java 685439e
> core/src/main/java/org/apache/sqoop/driver/Driver.java f1b45bb
> 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 c888910
> 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
>
> Diff: https://reviews.apache.org/r/26941/diff/
>
>
> Testing
> -------
>
> yes
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 26941: SQOOP-1557: SQ_CONFIGURABLE ( for entities
who own configs)
Posted by Veena Basavaraj <vb...@cloudera.com>.
> 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
>
>
Re: Review Request 26941: SQOOP-1557: SQ_CONFIGURABLE ( for entities
who own configs)
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26941/#review57652
-----------------------------------------------------------
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java
<https://reviews.apache.org/r/26941/#comment98453>
Super nit: Can we also put the semicolon on it's own line? :)
My goal here is to have ability to add new exception code by just adding a new line without need to touch other lines which will make it easier for "git cherrypick" or "git blame".
common/src/main/java/org/apache/sqoop/model/MConnector.java
<https://reviews.apache.org/r/26941/#comment98454>
Seems as still the same invalid import? :)
common/src/main/java/org/apache/sqoop/model/MDriver.java
<https://reviews.apache.org/r/26941/#comment98455>
Thank you for cleaning this one!
core/src/main/java/org/apache/sqoop/driver/Driver.java
<https://reviews.apache.org/r/26941/#comment98456>
Do you think that it would make sense to change this call to Driver.getClass().getSimpleName() so that we don't have to change it if at some point in the future we decides to change the package or class name?
core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java
<https://reviews.apache.org/r/26941/#comment98457>
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.
core/src/main/java/org/apache/sqoop/repository/Repository.java
<https://reviews.apache.org/r/26941/#comment98452>
Seems as unused import?
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
<https://reviews.apache.org/r/26941/#comment98458>
Can we keep the comment here so that it's obvious what we're doing here?
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java
<https://reviews.apache.org/r/26941/#comment98459>
Nit: Trailing whitespace
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java
<https://reviews.apache.org/r/26941/#comment98460>
Nit: Trailing whitespace
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java
<https://reviews.apache.org/r/26941/#comment98461>
Nit: Trailing whitespace
repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/DerbyTestCase.java
<https://reviews.apache.org/r/26941/#comment98462>
Nit: Trailing whitespace
Jarcec
- Jarek Cecho
On Oct. 21, 2014, 7: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, 7: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
>
>
Re: Review Request 26941: SQOOP-1557: SQ_CONFIGURABLE ( for entities
who own configs)
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26941/#review57739
-----------------------------------------------------------
Ship it!
Ship It!
- Jarek Cecho
On Oct. 21, 2014, 10: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, 10: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
>
>
Re: Review Request 26941: SQOOP-1557: SQ_CONFIGURABLE ( for entities
who own configs)
Posted by Veena Basavaraj <vb...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26941/
-----------------------------------------------------------
(Updated Oct. 21, 2014, 3:19 p.m.)
Review request for Sqoop.
Changes
-------
fix tests that was failing with harcoded Driver name
Repository: sqoop-sqoop2
Description
-------
see JIRA for details
there is whitespace, that will be addressed once the reviews for the functionality
Diffs (updated)
-----
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
Re: Review Request 26941: SQOOP-1557: SQ_CONFIGURABLE ( for entities
who own configs)
Posted by Veena Basavaraj <vb...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26941/
-----------------------------------------------------------
(Updated Oct. 21, 2014, 3:06 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 (updated)
-----
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
Re: Review Request 26941: SQOOP-1557: SQ_CONFIGURABLE ( for entities
who own configs)
Posted by Veena Basavaraj <vb...@cloudera.com>.
-----------------------------------------------------------
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.
Changes
-------
after rebase and ws removed
Repository: sqoop-sqoop2
Description
-------
see JIRA for details
there is whitespace, that will be addressed once the reviews for the functionality
Diffs (updated)
-----
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
Re: Review Request 26941: SQOOP-1557: SQ_CONFIGURABLE ( for entities
who own configs)
Posted by Veena Basavaraj <vb...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26941/
-----------------------------------------------------------
(Updated Oct. 21, 2014, 11:25 a.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 (updated)
-----
common/src/main/java/org/apache/sqoop/model/MConfigurableType.java PRE-CREATION
common/src/main/java/org/apache/sqoop/model/MConnector.java 2f42191
common/src/main/java/org/apache/sqoop/model/MDriver.java 685439e
core/src/main/java/org/apache/sqoop/driver/Driver.java f1b45bb
core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java f06fd0c
core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 5a8e026
core/src/main/java/org/apache/sqoop/repository/Repository.java 74a9e12
core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java 34bd8a5
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 c888910
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 95fbe07
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 76ebd3b
Diff: https://reviews.apache.org/r/26941/diff/
Testing
-------
yes
Thanks,
Veena Basavaraj
Re: Review Request 26941: SQOOP-1557: SQ_CONFIGURABLE ( for entities
who own configs)
Posted by Veena Basavaraj <vb...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26941/
-----------------------------------------------------------
(Updated Oct. 20, 2014, 11:27 a.m.)
Review request for Sqoop.
Repository: sqoop-sqoop2
Description (updated)
-------
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/MConfigurableType.java PRE-CREATION
common/src/main/java/org/apache/sqoop/model/MConnector.java 2f42191
common/src/main/java/org/apache/sqoop/model/MDriver.java 685439e
core/src/main/java/org/apache/sqoop/driver/Driver.java f1b45bb
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 c888910
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
Diff: https://reviews.apache.org/r/26941/diff/
Testing
-------
yes
Thanks,
Veena Basavaraj
Re: Review Request 26941: SQOOP-1557: SQ_CONFIGURABLE ( for entities
who own configs)
Posted by Jarek Cecho <ja...@apache.org>.
> On Oct. 20, 2014, 6:26 p.m., Jarek Cecho wrote:
> > Overall I like the changes. I do have couple of high level notes:
> >
> > 1) Please do remove all the trailing whitespaces. I've mark some of them. Most of the IDEs do have option to automatically remove trailing whispaces on file save, so you might want to enable that option :)
> >
> > 2) It seems that this patch also incorporates quite a lot of changes into the query names and pretty much refactores the Query class. I don't mind doing those changes, but they should be covered by a separate JIRA as they are unrelated to "rename SQ_CONFIGURABLE".
>
> Veena Basavaraj wrote:
> WS yes, I dont usually give it much thought until the end of the review, I do have it enable din IDE,
>
> While I was doing the upgrade it was really hard for me to follow things, when people just add things without much documentation.
>
> If you want another RB, I will spend time on it, splitting this up, but I dont want to scarifice code readability for some superficial deadline, so I will split it up.
Having proper JIRA per problem and not commit "uber JIRAs" is not for code redability nor to meet some deadlines. It's for us to enable tools such as "git cherry-pick" or "git blame" effectively which in the long term is must have based on my experience.
> On Oct. 20, 2014, 6:26 p.m., Jarek Cecho wrote:
> > common/src/main/java/org/apache/sqoop/model/MDriver.java, lines 71-73
> > <https://reviews.apache.org/r/26941/diff/1/?file=725833#file725833line71>
> >
> > Why returning String, wouldn't it make more sense to return the enumb MConfigurableType directly?
>
> Veena Basavaraj wrote:
> this method is most used to persist the value in database. I dont like hardcoding int he derby, like most other places have done esp cial for the config types.
>
> If we have more repository implementaitons tomm, we can just use the getType method.
I'm not following why would one use of this method in Derby database yeild the need to return String? Why can't we return enum that will be subsequently converted to String by Derby itself?
In my mind the model classes are very general as anyone can use them (they are in common package after all), so I'm not particularly found of the idea to design the API for one specific use case. I would assume that on calling getType(), I would get a type that I can easily compare (if statement) or put into switch statement, so I really don't see use case for String here.
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26941/#review57377
-----------------------------------------------------------
On Oct. 20, 2014, 6:27 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26941/
> -----------------------------------------------------------
>
> (Updated Oct. 20, 2014, 6:27 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/MConfigurableType.java PRE-CREATION
> common/src/main/java/org/apache/sqoop/model/MConnector.java 2f42191
> common/src/main/java/org/apache/sqoop/model/MDriver.java 685439e
> core/src/main/java/org/apache/sqoop/driver/Driver.java f1b45bb
> 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 c888910
> 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
>
> Diff: https://reviews.apache.org/r/26941/diff/
>
>
> Testing
> -------
>
> yes
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 26941: SQOOP-1557: SQ_CONFIGURABLE ( for entities
who own configs)
Posted by Veena Basavaraj <vb...@cloudera.com>.
> On Oct. 20, 2014, 11:26 a.m., Jarek Cecho wrote:
> > Overall I like the changes. I do have couple of high level notes:
> >
> > 1) Please do remove all the trailing whitespaces. I've mark some of them. Most of the IDEs do have option to automatically remove trailing whispaces on file save, so you might want to enable that option :)
> >
> > 2) It seems that this patch also incorporates quite a lot of changes into the query names and pretty much refactores the Query class. I don't mind doing those changes, but they should be covered by a separate JIRA as they are unrelated to "rename SQ_CONFIGURABLE".
>
> Veena Basavaraj wrote:
> WS yes, I dont usually give it much thought until the end of the review, I do have it enable din IDE,
>
> While I was doing the upgrade it was really hard for me to follow things, when people just add things without much documentation.
>
> If you want another RB, I will spend time on it, splitting this up, but I dont want to scarifice code readability for some superficial deadline, so I will split it up.
>
> Jarek Cecho wrote:
> Having proper JIRA per problem and not commit "uber JIRAs" is not for code redability nor to meet some deadlines. It's for us to enable tools such as "git cherry-pick" or "git blame" effectively which in the long term is must have based on my experience.
sure have created a ticket for documenting the derby schema code, migh as well split it up into classes for create / upgrade and the CRUD operations since you are open to it now.
> On Oct. 20, 2014, 11:26 a.m., Jarek Cecho wrote:
> > common/src/main/java/org/apache/sqoop/model/MDriver.java, lines 71-73
> > <https://reviews.apache.org/r/26941/diff/1/?file=725833#file725833line71>
> >
> > Why returning String, wouldn't it make more sense to return the enumb MConfigurableType directly?
>
> Veena Basavaraj wrote:
> this method is most used to persist the value in database. I dont like hardcoding int he derby, like most other places have done esp cial for the config types.
>
> If we have more repository implementaitons tomm, we can just use the getType method.
>
> Jarek Cecho wrote:
> I'm not following why would one use of this method in Derby database yeild the need to return String? Why can't we return enum that will be subsequently converted to String by Derby itself?
>
> In my mind the model classes are very general as anyone can use them (they are in common package after all), so I'm not particularly found of the idea to design the API for one specific use case. I would assume that on calling getType(), I would get a type that I can easily compare (if statement) or put into switch statement, so I really don't see use case for String here.
Sure. I think it makes sense to have Enum. changed.
- Veena
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26941/#review57377
-----------------------------------------------------------
On Oct. 20, 2014, 11:27 a.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26941/
> -----------------------------------------------------------
>
> (Updated Oct. 20, 2014, 11:27 a.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/MConfigurableType.java PRE-CREATION
> common/src/main/java/org/apache/sqoop/model/MConnector.java 2f42191
> common/src/main/java/org/apache/sqoop/model/MDriver.java 685439e
> core/src/main/java/org/apache/sqoop/driver/Driver.java f1b45bb
> 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 c888910
> 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
>
> Diff: https://reviews.apache.org/r/26941/diff/
>
>
> Testing
> -------
>
> yes
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 26941: SQOOP-1557: SQ_CONFIGURABLE ( for entities
who own configs)
Posted by Veena Basavaraj <vb...@cloudera.com>.
> On Oct. 20, 2014, 11:26 a.m., Jarek Cecho wrote:
> > Overall I like the changes. I do have couple of high level notes:
> >
> > 1) Please do remove all the trailing whitespaces. I've mark some of them. Most of the IDEs do have option to automatically remove trailing whispaces on file save, so you might want to enable that option :)
> >
> > 2) It seems that this patch also incorporates quite a lot of changes into the query names and pretty much refactores the Query class. I don't mind doing those changes, but they should be covered by a separate JIRA as they are unrelated to "rename SQ_CONFIGURABLE".
WS yes, I dont usually give it much thought until the end of the review, I do have it enable din IDE,
While I was doing the upgrade it was really hard for me to follow things, when people just add things without much documentation.
If you want another RB, I will spend time on it, splitting this up, but I dont want to scarifice code readability for some superficial deadline, so I will split it up.
- Veena
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26941/#review57377
-----------------------------------------------------------
On Oct. 20, 2014, 11:27 a.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26941/
> -----------------------------------------------------------
>
> (Updated Oct. 20, 2014, 11:27 a.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/MConfigurableType.java PRE-CREATION
> common/src/main/java/org/apache/sqoop/model/MConnector.java 2f42191
> common/src/main/java/org/apache/sqoop/model/MDriver.java 685439e
> core/src/main/java/org/apache/sqoop/driver/Driver.java f1b45bb
> 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 c888910
> 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
>
> Diff: https://reviews.apache.org/r/26941/diff/
>
>
> Testing
> -------
>
> yes
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 26941: SQOOP-1557: SQ_CONFIGURABLE ( for entities
who own configs)
Posted by Veena Basavaraj <vb...@cloudera.com>.
> On Oct. 20, 2014, 11:26 a.m., Jarek Cecho wrote:
> > common/src/main/java/org/apache/sqoop/model/MDriver.java, lines 71-73
> > <https://reviews.apache.org/r/26941/diff/1/?file=725833#file725833line71>
> >
> > Why returning String, wouldn't it make more sense to return the enumb MConfigurableType directly?
this method is most used to persist the value in database. I dont like hardcoding int he derby, like most other places have done esp cial for the config types.
If we have more repository implementaitons tomm, we can just use the getType method.
> On Oct. 20, 2014, 11:26 a.m., Jarek Cecho wrote:
> > common/src/main/java/org/apache/sqoop/model/MConnector.java, line 20
> > <https://reviews.apache.org/r/26941/diff/1/?file=725832#file725832line20>
> >
> > Seems as irrelevant import?
I have no idea why this happens:) fixed
- Veena
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26941/#review57377
-----------------------------------------------------------
On Oct. 20, 2014, 11:27 a.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26941/
> -----------------------------------------------------------
>
> (Updated Oct. 20, 2014, 11:27 a.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/MConfigurableType.java PRE-CREATION
> common/src/main/java/org/apache/sqoop/model/MConnector.java 2f42191
> common/src/main/java/org/apache/sqoop/model/MDriver.java 685439e
> core/src/main/java/org/apache/sqoop/driver/Driver.java f1b45bb
> 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 c888910
> 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
>
> Diff: https://reviews.apache.org/r/26941/diff/
>
>
> Testing
> -------
>
> yes
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 26941: SQOOP-1557: SQ_CONFIGURABLE ( for entities
who own configs)
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26941/#review57377
-----------------------------------------------------------
Overall I like the changes. I do have couple of high level notes:
1) Please do remove all the trailing whitespaces. I've mark some of them. Most of the IDEs do have option to automatically remove trailing whispaces on file save, so you might want to enable that option :)
2) It seems that this patch also incorporates quite a lot of changes into the query names and pretty much refactores the Query class. I don't mind doing those changes, but they should be covered by a separate JIRA as they are unrelated to "rename SQ_CONFIGURABLE".
common/src/main/java/org/apache/sqoop/model/MConnector.java
<https://reviews.apache.org/r/26941/#comment98050>
Seems as irrelevant import?
common/src/main/java/org/apache/sqoop/model/MConnector.java
<https://reviews.apache.org/r/26941/#comment98052>
Why returning String? Would be better to return MConfigurableType directly?
common/src/main/java/org/apache/sqoop/model/MDriver.java
<https://reviews.apache.org/r/26941/#comment98053>
Nit: Trailing white space
common/src/main/java/org/apache/sqoop/model/MDriver.java
<https://reviews.apache.org/r/26941/#comment98055>
Why returning String, wouldn't it make more sense to return the enumb MConfigurableType directly?
common/src/main/java/org/apache/sqoop/model/MDriver.java
<https://reviews.apache.org/r/26941/#comment98054>
Nit: Trailing white space
core/src/main/java/org/apache/sqoop/driver/Driver.java
<https://reviews.apache.org/r/26941/#comment98056>
Nit: trailing whitespace
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java
<https://reviews.apache.org/r/26941/#comment98057>
Nit : Whitespace error
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java
<https://reviews.apache.org/r/26941/#comment98058>
Super nit:
Please put a comma at the end of the line and semicolon into it's own line. This way adding a new error message means to add a new line instead of changing the existing line as well. This will make tools such as git blame or git cherrypick much easier to use.
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
<https://reviews.apache.org/r/26941/#comment98059>
Nit: Trailing whitespace
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
<https://reviews.apache.org/r/26941/#comment98060>
Nit: Trailing whitespace
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
<https://reviews.apache.org/r/26941/#comment98061>
Nit: Trailing whitespace
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
<https://reviews.apache.org/r/26941/#comment98062>
Nit: Trailing whitespace
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaConstants.java
<https://reviews.apache.org/r/26941/#comment98063>
Nit: Trailing whitespace
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaConstants.java
<https://reviews.apache.org/r/26941/#comment98064>
Nit: Trailing whitespace
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java
<https://reviews.apache.org/r/26941/#comment98065>
Nit: Trailing whitespace
Jarcec
- Jarek Cecho
On Oct. 20, 2014, 4:46 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26941/
> -----------------------------------------------------------
>
> (Updated Oct. 20, 2014, 4:46 p.m.)
>
>
> Review request for Sqoop.
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see JIRA for details
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/model/MConfigurableType.java PRE-CREATION
> common/src/main/java/org/apache/sqoop/model/MConnector.java 2f42191
> common/src/main/java/org/apache/sqoop/model/MDriver.java 685439e
> core/src/main/java/org/apache/sqoop/driver/Driver.java f1b45bb
> 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 c888910
> 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
>
> Diff: https://reviews.apache.org/r/26941/diff/
>
>
> Testing
> -------
>
> yes
>
>
> Thanks,
>
> Veena Basavaraj
>
>