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/24 16:14:58 UTC
Review Request 27137: SQOOP-1597: Refactor DerbySchemaQuery into one
for create/ update/ and then CRUD operarations
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27137/
-----------------------------------------------------------
Review request for Sqoop.
Repository: sqoop-sqoop2
Description
-------
see jira for details.
Diffs
-----
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 633e9df
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaCreateQuery.java PRE-CREATION
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java PRE-CREATION
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java ce1830e
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaUpgradeQuery.java PRE-CREATION
repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/DerbyTestCase.java b22dbd5
Diff: https://reviews.apache.org/r/27137/diff/
Testing
-------
yes
Thanks,
Veena Basavaraj
Re: Review Request 27137: SQOOP-1597: Refactor DerbySchemaQuery into
one for create/ update/ and then CRUD operarations
Posted by Veena Basavaraj <vb...@cloudera.com>.
> On Oct. 24, 2014, 1:55 p.m., Abraham Elmahrek wrote:
> > With SQOOP-1589, I think things will be split up a bit. Should we wait for SQOOP-1589?
>
> Veena Basavaraj wrote:
> I did see that change, all this does it makes it easy for some newbie to see what is create and what is update and what is CRUD. I had a general comment on 1589 after I took a look at it, queries are something we should move out of the java code in the long run
>
> So postgres should start with it
Also one of the primary reason for splitting up this is to avoid a lot of merge conflicts amongst ourselves.!
- Veena
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27137/#review58365
-----------------------------------------------------------
On Oct. 24, 2014, 7:32 a.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27137/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2014, 7:32 a.m.)
>
>
> Review request for Sqoop.
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see jira for details.
>
>
> Diffs
> -----
>
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 633e9df
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaCreateQuery.java PRE-CREATION
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java PRE-CREATION
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java ce1830e
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaUpgradeQuery.java PRE-CREATION
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/DerbyTestCase.java b22dbd5
>
> Diff: https://reviews.apache.org/r/27137/diff/
>
>
> Testing
> -------
>
> yes
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 27137: SQOOP-1597: Refactor DerbySchemaQuery into
one for create/ update/ and then CRUD operarations
Posted by Veena Basavaraj <vb...@cloudera.com>.
> On Oct. 24, 2014, 1:55 p.m., Abraham Elmahrek wrote:
> > With SQOOP-1589, I think things will be split up a bit. Should we wait for SQOOP-1589?
I did see that change, all this does it makes it easy for some newbie to see what is create and what is update and what is CRUD. I had a general comment on 1589 after I took a look at it, queries are something we should move out of the java code in the long run
So postgres should start with it
- Veena
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27137/#review58365
-----------------------------------------------------------
On Oct. 24, 2014, 7:32 a.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27137/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2014, 7:32 a.m.)
>
>
> Review request for Sqoop.
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see jira for details.
>
>
> Diffs
> -----
>
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 633e9df
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaCreateQuery.java PRE-CREATION
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java PRE-CREATION
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java ce1830e
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaUpgradeQuery.java PRE-CREATION
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/DerbyTestCase.java b22dbd5
>
> Diff: https://reviews.apache.org/r/27137/diff/
>
>
> Testing
> -------
>
> yes
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 27137: SQOOP-1597: Refactor DerbySchemaQuery into
one for create/ update/ and then CRUD operarations
Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27137/#review58365
-----------------------------------------------------------
With SQOOP-1589, I think things will be split up a bit. Should we wait for SQOOP-1589?
- Abraham Elmahrek
On Oct. 24, 2014, 2:32 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27137/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2014, 2:32 p.m.)
>
>
> Review request for Sqoop.
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see jira for details.
>
>
> Diffs
> -----
>
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 633e9df
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaCreateQuery.java PRE-CREATION
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java PRE-CREATION
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java ce1830e
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaUpgradeQuery.java PRE-CREATION
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/DerbyTestCase.java b22dbd5
>
> Diff: https://reviews.apache.org/r/27137/diff/
>
>
> Testing
> -------
>
> yes
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 27137: SQOOP-1597: Refactor DerbySchemaQuery into
one for create/ update/ and then CRUD operarations
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27137/#review58686
-----------------------------------------------------------
Ship it!
Ship It!
- Jarek Cecho
On Oct. 27, 2014, 8:59 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27137/
> -----------------------------------------------------------
>
> (Updated Oct. 27, 2014, 8:59 p.m.)
>
>
> Review request for Sqoop.
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see jira for details.
>
>
> Diffs
> -----
>
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java b4b5f3e
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaCreateQuery.java PRE-CREATION
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java PRE-CREATION
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java 0a5b467
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaUpgradeQuery.java PRE-CREATION
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/DerbyTestCase.java 528653c
>
> Diff: https://reviews.apache.org/r/27137/diff/
>
>
> Testing
> -------
>
> yes
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 27137: SQOOP-1597: Refactor DerbySchemaQuery into
one for create/ update/ and then CRUD operarations
Posted by Veena Basavaraj <vb...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27137/
-----------------------------------------------------------
(Updated Oct. 27, 2014, 1:59 p.m.)
Review request for Sqoop.
Repository: sqoop-sqoop2
Description
-------
see jira for details.
Diffs (updated)
-----
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java b4b5f3e
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaCreateQuery.java PRE-CREATION
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java PRE-CREATION
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java 0a5b467
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaUpgradeQuery.java PRE-CREATION
repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/DerbyTestCase.java 528653c
Diff: https://reviews.apache.org/r/27137/diff/
Testing
-------
yes
Thanks,
Veena Basavaraj
Re: Review Request 27137: SQOOP-1597: Refactor DerbySchemaQuery into
one for create/ update/ and then CRUD operarations
Posted by Jarek Cecho <ja...@apache.org>.
> On Oct. 26, 2014, 4:41 a.m., Qian Xu wrote:
> > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java, line 20
> > <https://reviews.apache.org/r/27137/diff/2/?file=732162#file732162line20>
> >
> > Better expand imports (this may require to change IDE's optimize import settings)
I don't like the astersisk import either, but in this case it seems reasonable as those two files are very tighly connected.
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27137/#review58553
-----------------------------------------------------------
On Oct. 27, 2014, 8:59 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27137/
> -----------------------------------------------------------
>
> (Updated Oct. 27, 2014, 8:59 p.m.)
>
>
> Review request for Sqoop.
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see jira for details.
>
>
> Diffs
> -----
>
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java b4b5f3e
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaCreateQuery.java PRE-CREATION
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java PRE-CREATION
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java 0a5b467
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaUpgradeQuery.java PRE-CREATION
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/DerbyTestCase.java 528653c
>
> Diff: https://reviews.apache.org/r/27137/diff/
>
>
> Testing
> -------
>
> yes
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 27137: SQOOP-1597: Refactor DerbySchemaQuery into
one for create/ update/ and then CRUD operarations
Posted by Veena Basavaraj <vb...@cloudera.com>.
> On Oct. 25, 2014, 9:41 p.m., Qian Xu wrote:
> > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java, line 20
> > <https://reviews.apache.org/r/27137/diff/2/?file=732162#file732162line20>
> >
> > Looks good, only one suggestion: better expand imports (Probably disable IDE's "optimize imports" settings)
we should come up with a code convention for these, this was intentional to collapse it since every single of them is imported
- Veena
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27137/#review58553
-----------------------------------------------------------
On Oct. 24, 2014, 7:32 a.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27137/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2014, 7:32 a.m.)
>
>
> Review request for Sqoop.
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see jira for details.
>
>
> Diffs
> -----
>
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 633e9df
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaCreateQuery.java PRE-CREATION
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java PRE-CREATION
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java ce1830e
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaUpgradeQuery.java PRE-CREATION
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/DerbyTestCase.java b22dbd5
>
> Diff: https://reviews.apache.org/r/27137/diff/
>
>
> Testing
> -------
>
> yes
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 27137: SQOOP-1597: Refactor DerbySchemaQuery into
one for create/ update/ and then CRUD operarations
Posted by Qian Xu <sx...@googlemail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27137/#review58553
-----------------------------------------------------------
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
<https://reviews.apache.org/r/27137/#comment99622>
Better expand imports (this may require to change IDE's optimize import settings)
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
<https://reviews.apache.org/r/27137/#comment99623>
Looks good, only one suggestion: better expand imports (Probably disable IDE's "optimize imports" settings)
- Qian Xu
On Oct. 24, 2014, 10:32 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27137/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2014, 10:32 p.m.)
>
>
> Review request for Sqoop.
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see jira for details.
>
>
> Diffs
> -----
>
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 633e9df
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaCreateQuery.java PRE-CREATION
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java PRE-CREATION
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java ce1830e
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaUpgradeQuery.java PRE-CREATION
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/DerbyTestCase.java b22dbd5
>
> Diff: https://reviews.apache.org/r/27137/diff/
>
>
> Testing
> -------
>
> yes
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 27137: SQOOP-1597: Refactor DerbySchemaQuery into
one for create/ update/ and then CRUD operarations
Posted by Veena Basavaraj <vb...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27137/
-----------------------------------------------------------
(Updated Oct. 24, 2014, 7:32 a.m.)
Review request for Sqoop.
Changes
-------
remove WS ( there is no functionality change)
Repository: sqoop-sqoop2
Description
-------
see jira for details.
Diffs (updated)
-----
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 633e9df
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaCreateQuery.java PRE-CREATION
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java PRE-CREATION
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java ce1830e
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaUpgradeQuery.java PRE-CREATION
repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/DerbyTestCase.java b22dbd5
Diff: https://reviews.apache.org/r/27137/diff/
Testing
-------
yes
Thanks,
Veena Basavaraj