You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Abraham Elmahrek <ab...@cloudera.com> on 2013/09/26 22:02:41 UTC
Review Request 14351: SQOOP-1208 Sqoop2: job ID column is too small
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14351/
-----------------------------------------------------------
Review request for Sqoop, Hari Shreedharan and Jarek Cecho.
Bugs: SQOOP-1208
https://issues.apache.org/jira/browse/SQOOP-1208
Repository: sqoop-sqoop2
Description
-------
commit 2ee24e0ca20925523cb3c8a87d9c414bcdd80293
Author: Abraham Elmahrek <ab...@elmahrek.com>
Date: Thu Sep 26 12:59:42 2013 -0700
SQOOP-1208 Sqoop2: job ID column is too small
Increased it to 50
:100644 100644 0a196f0... 4184129... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java
Diffs
-----
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java 0a196f0
Diff: https://reviews.apache.org/r/14351/diff/
Testing
-------
Ran sqoop tests and verified fix locally.
Thanks,
Abraham Elmahrek
Re: Review Request 14351: SQOOP-1208 Sqoop2: job ID column is too small
Posted by Abraham Elmahrek <ab...@cloudera.com>.
> On Oct. 2, 2013, 1:08 a.m., Jarek Cecho wrote:
> > Ship It!
Thanks Jarcec!
- Abraham
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14351/#review26589
-----------------------------------------------------------
On Oct. 1, 2013, 8:45 p.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14351/
> -----------------------------------------------------------
>
> (Updated Oct. 1, 2013, 8:45 p.m.)
>
>
> Review request for Sqoop, Hari Shreedharan and Jarek Cecho.
>
>
> Bugs: SQOOP-1208
> https://issues.apache.org/jira/browse/SQOOP-1208
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit eb0e73f08c80859c979aac17cf8df8522b43368a
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date: Thu Sep 26 12:59:42 2013 -0700
>
> SQOOP-1208 Sqoop2: job ID column is too small
>
> Increased it to 50
>
> :100644 100644 bb01f03... 200293d... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoConstants.java
> :100644 100644 f6bd5b5... 331780b... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
> :100644 100644 0a196f0... b86d5f8... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java
>
>
> Diffs
> -----
>
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoConstants.java bb01f0308499145e20ebebae1473591ba17227d0
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java f6bd5b5c560486f85db0676ae74544a8c8420b51
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java 0a196f03a39fd0c802de643988b73fc804ec68e1
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/DerbyTestCase.java a2a0bc73d5cc27a2ce694f09fc5a52b90ad1bd1f
>
> Diff: https://reviews.apache.org/r/14351/diff/
>
>
> Testing
> -------
>
> Ran sqoop tests and verified fix locally.
>
>
> Thanks,
>
> Abraham Elmahrek
>
>
Re: Review Request 14351: SQOOP-1208 Sqoop2: job ID column is too small
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14351/#review26589
-----------------------------------------------------------
Ship it!
Ship It!
- Jarek Cecho
On Oct. 1, 2013, 8:45 p.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14351/
> -----------------------------------------------------------
>
> (Updated Oct. 1, 2013, 8:45 p.m.)
>
>
> Review request for Sqoop, Hari Shreedharan and Jarek Cecho.
>
>
> Bugs: SQOOP-1208
> https://issues.apache.org/jira/browse/SQOOP-1208
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit eb0e73f08c80859c979aac17cf8df8522b43368a
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date: Thu Sep 26 12:59:42 2013 -0700
>
> SQOOP-1208 Sqoop2: job ID column is too small
>
> Increased it to 50
>
> :100644 100644 bb01f03... 200293d... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoConstants.java
> :100644 100644 f6bd5b5... 331780b... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
> :100644 100644 0a196f0... b86d5f8... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java
>
>
> Diffs
> -----
>
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoConstants.java bb01f0308499145e20ebebae1473591ba17227d0
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java f6bd5b5c560486f85db0676ae74544a8c8420b51
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java 0a196f03a39fd0c802de643988b73fc804ec68e1
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/DerbyTestCase.java a2a0bc73d5cc27a2ce694f09fc5a52b90ad1bd1f
>
> Diff: https://reviews.apache.org/r/14351/diff/
>
>
> Testing
> -------
>
> Ran sqoop tests and verified fix locally.
>
>
> Thanks,
>
> Abraham Elmahrek
>
>
Re: Review Request 14351: SQOOP-1208 Sqoop2: job ID column is too small
Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14351/
-----------------------------------------------------------
(Updated Oct. 1, 2013, 8:45 p.m.)
Review request for Sqoop, Hari Shreedharan and Jarek Cecho.
Changes
-------
Ran tests and integration tests.
Bugs: SQOOP-1208
https://issues.apache.org/jira/browse/SQOOP-1208
Repository: sqoop-sqoop2
Description
-------
commit eb0e73f08c80859c979aac17cf8df8522b43368a
Author: Abraham Elmahrek <ab...@elmahrek.com>
Date: Thu Sep 26 12:59:42 2013 -0700
SQOOP-1208 Sqoop2: job ID column is too small
Increased it to 50
:100644 100644 bb01f03... 200293d... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoConstants.java
:100644 100644 f6bd5b5... 331780b... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
:100644 100644 0a196f0... b86d5f8... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java
Diffs (updated)
-----
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoConstants.java bb01f0308499145e20ebebae1473591ba17227d0
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java f6bd5b5c560486f85db0676ae74544a8c8420b51
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java 0a196f03a39fd0c802de643988b73fc804ec68e1
repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/DerbyTestCase.java a2a0bc73d5cc27a2ce694f09fc5a52b90ad1bd1f
Diff: https://reviews.apache.org/r/14351/diff/
Testing
-------
Ran sqoop tests and verified fix locally.
Thanks,
Abraham Elmahrek
Re: Review Request 14351: SQOOP-1208 Sqoop2: job ID column is too small
Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14351/
-----------------------------------------------------------
(Updated Oct. 1, 2013, 6:59 a.m.)
Review request for Sqoop, Hari Shreedharan and Jarek Cecho.
Bugs: SQOOP-1208
https://issues.apache.org/jira/browse/SQOOP-1208
Repository: sqoop-sqoop2
Description
-------
commit eb0e73f08c80859c979aac17cf8df8522b43368a
Author: Abraham Elmahrek <ab...@elmahrek.com>
Date: Thu Sep 26 12:59:42 2013 -0700
SQOOP-1208 Sqoop2: job ID column is too small
Increased it to 50
:100644 100644 bb01f03... 200293d... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoConstants.java
:100644 100644 f6bd5b5... 331780b... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
:100644 100644 0a196f0... b86d5f8... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java
Diffs (updated)
-----
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoConstants.java bb01f0308499145e20ebebae1473591ba17227d0
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java f6bd5b5c560486f85db0676ae74544a8c8420b51
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java 0a196f03a39fd0c802de643988b73fc804ec68e1
Diff: https://reviews.apache.org/r/14351/diff/
Testing
-------
Ran sqoop tests and verified fix locally.
Thanks,
Abraham Elmahrek
Re: Review Request 14351: SQOOP-1208 Sqoop2: job ID column is too small
Posted by Abraham Elmahrek <ab...@cloudera.com>.
> On Oct. 1, 2013, 2:13 a.m., Jarek Cecho wrote:
> > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java, lines 281-293
> > <https://reviews.apache.org/r/14351/diff/2/?file=359460#file359460line281>
> >
> > I think that this change is not necessary as the version "2" will be correctly detected in "normal version detection" block that is starting just after this block that is there specifically for pre 1.99.2 repository structures, right?
In the normal detect version function? I don't think so. In the general getVersion code? Yes, but I isn't better to explicitly define versions when managing database state?
- Abraham
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14351/#review26538
-----------------------------------------------------------
On Sept. 27, 2013, 11:02 p.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14351/
> -----------------------------------------------------------
>
> (Updated Sept. 27, 2013, 11:02 p.m.)
>
>
> Review request for Sqoop, Hari Shreedharan and Jarek Cecho.
>
>
> Bugs: SQOOP-1208
> https://issues.apache.org/jira/browse/SQOOP-1208
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit eb0e73f08c80859c979aac17cf8df8522b43368a
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date: Thu Sep 26 12:59:42 2013 -0700
>
> SQOOP-1208 Sqoop2: job ID column is too small
>
> Increased it to 50
>
> :100644 100644 bb01f03... 200293d... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoConstants.java
> :100644 100644 f6bd5b5... 331780b... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
> :100644 100644 0a196f0... b86d5f8... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java
>
>
> Diffs
> -----
>
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoConstants.java bb01f0308499145e20ebebae1473591ba17227d0
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java f6bd5b5c560486f85db0676ae74544a8c8420b51
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java 0a196f03a39fd0c802de643988b73fc804ec68e1
>
> Diff: https://reviews.apache.org/r/14351/diff/
>
>
> Testing
> -------
>
> Ran sqoop tests and verified fix locally.
>
>
> Thanks,
>
> Abraham Elmahrek
>
>
Re: Review Request 14351: SQOOP-1208 Sqoop2: job ID column is too small
Posted by Abraham Elmahrek <ab...@cloudera.com>.
> On Oct. 1, 2013, 2:13 a.m., Jarek Cecho wrote:
> > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java, lines 281-293
> > <https://reviews.apache.org/r/14351/diff/2/?file=359460#file359460line281>
> >
> > I think that this change is not necessary as the version "2" will be correctly detected in "normal version detection" block that is starting just after this block that is there specifically for pre 1.99.2 repository structures, right?
>
> Abraham Elmahrek wrote:
> In the normal detect version function? I don't think so. In the general getVersion code? Yes, but I isn't better to explicitly define versions when managing database state?
I see your point now Jarcec! I will revert this code.
- Abraham
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14351/#review26538
-----------------------------------------------------------
On Sept. 27, 2013, 11:02 p.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14351/
> -----------------------------------------------------------
>
> (Updated Sept. 27, 2013, 11:02 p.m.)
>
>
> Review request for Sqoop, Hari Shreedharan and Jarek Cecho.
>
>
> Bugs: SQOOP-1208
> https://issues.apache.org/jira/browse/SQOOP-1208
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit eb0e73f08c80859c979aac17cf8df8522b43368a
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date: Thu Sep 26 12:59:42 2013 -0700
>
> SQOOP-1208 Sqoop2: job ID column is too small
>
> Increased it to 50
>
> :100644 100644 bb01f03... 200293d... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoConstants.java
> :100644 100644 f6bd5b5... 331780b... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
> :100644 100644 0a196f0... b86d5f8... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java
>
>
> Diffs
> -----
>
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoConstants.java bb01f0308499145e20ebebae1473591ba17227d0
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java f6bd5b5c560486f85db0676ae74544a8c8420b51
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java 0a196f03a39fd0c802de643988b73fc804ec68e1
>
> Diff: https://reviews.apache.org/r/14351/diff/
>
>
> Testing
> -------
>
> Ran sqoop tests and verified fix locally.
>
>
> Thanks,
>
> Abraham Elmahrek
>
>
Re: Review Request 14351: SQOOP-1208 Sqoop2: job ID column is too small
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14351/#review26538
-----------------------------------------------------------
Hi Abe,
thank you very much for incorporating the upgrade path to the patch, appreciated! I do have one question:
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
<https://reviews.apache.org/r/14351/#comment51785>
I think that this change is not necessary as the version "2" will be correctly detected in "normal version detection" block that is starting just after this block that is there specifically for pre 1.99.2 repository structures, right?
Jarcec
- Jarek Cecho
On Sept. 27, 2013, 11:02 p.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14351/
> -----------------------------------------------------------
>
> (Updated Sept. 27, 2013, 11:02 p.m.)
>
>
> Review request for Sqoop, Hari Shreedharan and Jarek Cecho.
>
>
> Bugs: SQOOP-1208
> https://issues.apache.org/jira/browse/SQOOP-1208
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit eb0e73f08c80859c979aac17cf8df8522b43368a
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date: Thu Sep 26 12:59:42 2013 -0700
>
> SQOOP-1208 Sqoop2: job ID column is too small
>
> Increased it to 50
>
> :100644 100644 bb01f03... 200293d... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoConstants.java
> :100644 100644 f6bd5b5... 331780b... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
> :100644 100644 0a196f0... b86d5f8... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java
>
>
> Diffs
> -----
>
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoConstants.java bb01f0308499145e20ebebae1473591ba17227d0
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java f6bd5b5c560486f85db0676ae74544a8c8420b51
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java 0a196f03a39fd0c802de643988b73fc804ec68e1
>
> Diff: https://reviews.apache.org/r/14351/diff/
>
>
> Testing
> -------
>
> Ran sqoop tests and verified fix locally.
>
>
> Thanks,
>
> Abraham Elmahrek
>
>
Re: Review Request 14351: SQOOP-1208 Sqoop2: job ID column is too small
Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14351/
-----------------------------------------------------------
(Updated Sept. 27, 2013, 11:02 p.m.)
Review request for Sqoop, Hari Shreedharan and Jarek Cecho.
Bugs: SQOOP-1208
https://issues.apache.org/jira/browse/SQOOP-1208
Repository: sqoop-sqoop2
Description (updated)
-------
commit eb0e73f08c80859c979aac17cf8df8522b43368a
Author: Abraham Elmahrek <ab...@elmahrek.com>
Date: Thu Sep 26 12:59:42 2013 -0700
SQOOP-1208 Sqoop2: job ID column is too small
Increased it to 50
:100644 100644 bb01f03... 200293d... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoConstants.java
:100644 100644 f6bd5b5... 331780b... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
:100644 100644 0a196f0... b86d5f8... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java
Diffs (updated)
-----
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoConstants.java bb01f0308499145e20ebebae1473591ba17227d0
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java f6bd5b5c560486f85db0676ae74544a8c8420b51
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java 0a196f03a39fd0c802de643988b73fc804ec68e1
Diff: https://reviews.apache.org/r/14351/diff/
Testing
-------
Ran sqoop tests and verified fix locally.
Thanks,
Abraham Elmahrek
Re: Review Request 14351: SQOOP-1208 Sqoop2: job ID column is too small
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14351/#review26421
-----------------------------------------------------------
Hi Abe,
thank you very much for taking this one up! I believe that we also needs to cover upgrade path when one is updating derby repository from prior Sqoop versions. Would you mind adding that?
Jarcec
- Jarek Cecho
On Sept. 26, 2013, 8:02 p.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14351/
> -----------------------------------------------------------
>
> (Updated Sept. 26, 2013, 8:02 p.m.)
>
>
> Review request for Sqoop, Hari Shreedharan and Jarek Cecho.
>
>
> Bugs: SQOOP-1208
> https://issues.apache.org/jira/browse/SQOOP-1208
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit 2ee24e0ca20925523cb3c8a87d9c414bcdd80293
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date: Thu Sep 26 12:59:42 2013 -0700
>
> SQOOP-1208 Sqoop2: job ID column is too small
>
> Increased it to 50
>
> :100644 100644 0a196f0... 4184129... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java
>
>
> Diffs
> -----
>
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java 0a196f0
>
> Diff: https://reviews.apache.org/r/14351/diff/
>
>
> Testing
> -------
>
> Ran sqoop tests and verified fix locally.
>
>
> Thanks,
>
> Abraham Elmahrek
>
>