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 2014/10/23 19:13:20 UTC
Review Request 27085: SQOOP-1589: Sqoop2: Create common constants,
error codes, and queries
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27085/
-----------------------------------------------------------
Review request for Sqoop.
Bugs: SQOOP-158
https://issues.apache.org/jira/browse/SQOOP-158
Repository: sqoop-sqoop2
Description
-------
commit 370f476b1834c390993b31007b0fc7e1a184b891
Author: Abraham Elmahrek <ab...@elmahrek.com>
Date: Wed Oct 22 18:30:28 2014 -0700
SQOOP-1589: Sqoop2: Create common constants, error codes, and queries
:100644 100644 7d78826... e41cd6c... M core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
:000000 100644 0000000... a7ebd50... A core/src/main/java/org/apache/sqoop/repository/JdbcRepositorySchemaConstants.java
:000000 100644 0000000... eb05567... A core/src/main/java/org/apache/sqoop/repository/JdbcRepositorySchemaQuery.java
:100644 100644 f684e85... 097a185... M core/src/main/java/org/apache/sqoop/repository/RepositoryError.java
:100644 100644 aad219e... 769544b... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java
:100644 100644 633e9df... 6764b14... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
:100644 100644 ce1830e... ff8962c... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java
:100644 100644 85140d5... aab1e95... M repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java
:100644 100644 37343d3... 9bcf3c8... M repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java
Diffs
-----
core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 7d78826
core/src/main/java/org/apache/sqoop/repository/JdbcRepositorySchemaConstants.java PRE-CREATION
core/src/main/java/org/apache/sqoop/repository/JdbcRepositorySchemaQuery.java PRE-CREATION
core/src/main/java/org/apache/sqoop/repository/RepositoryError.java f684e85
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java aad219e
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/DerbySchemaQuery.java ce1830e
repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java 85140d5
repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java 37343d3
Diff: https://reviews.apache.org/r/27085/diff/
Testing
-------
mvn clean verify + start sqoop server without issues.
Thanks,
Abraham Elmahrek
Re: Review Request 27085: SQOOP-1589: Sqoop2: Create common constants,
error codes, and queries
Posted by Abraham Elmahrek <ab...@cloudera.com>.
> On Oct. 24, 2014, 5:15 a.m., Qian Xu wrote:
> >
Qian, the comments you've left are oriented around code that I haven't written or modified. Let's have follow up Jiras to address the issues you've brought up?
- Abraham
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27085/#review58243
-----------------------------------------------------------
On Oct. 23, 2014, 5:13 p.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27085/
> -----------------------------------------------------------
>
> (Updated Oct. 23, 2014, 5:13 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-158
> https://issues.apache.org/jira/browse/SQOOP-158
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit 370f476b1834c390993b31007b0fc7e1a184b891
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date: Wed Oct 22 18:30:28 2014 -0700
>
> SQOOP-1589: Sqoop2: Create common constants, error codes, and queries
>
> :100644 100644 7d78826... e41cd6c... M core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
> :000000 100644 0000000... a7ebd50... A core/src/main/java/org/apache/sqoop/repository/JdbcRepositorySchemaConstants.java
> :000000 100644 0000000... eb05567... A core/src/main/java/org/apache/sqoop/repository/JdbcRepositorySchemaQuery.java
> :100644 100644 f684e85... 097a185... M core/src/main/java/org/apache/sqoop/repository/RepositoryError.java
> :100644 100644 aad219e... 769544b... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java
> :100644 100644 633e9df... 6764b14... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
> :100644 100644 ce1830e... ff8962c... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java
> :100644 100644 85140d5... aab1e95... M repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java
> :100644 100644 37343d3... 9bcf3c8... M repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 7d78826
> core/src/main/java/org/apache/sqoop/repository/JdbcRepositorySchemaConstants.java PRE-CREATION
> core/src/main/java/org/apache/sqoop/repository/JdbcRepositorySchemaQuery.java PRE-CREATION
> core/src/main/java/org/apache/sqoop/repository/RepositoryError.java f684e85
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java aad219e
> 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/DerbySchemaQuery.java ce1830e
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java 85140d5
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java 37343d3
>
> Diff: https://reviews.apache.org/r/27085/diff/
>
>
> Testing
> -------
>
> mvn clean verify + start sqoop server without issues.
>
>
> Thanks,
>
> Abraham Elmahrek
>
>
Re: Review Request 27085: SQOOP-1589: Sqoop2: Create common constants,
error codes, and queries
Posted by Qian Xu <sx...@googlemail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27085/#review58243
-----------------------------------------------------------
core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
<https://reviews.apache.org/r/27085/#comment99194>
`LOG.isDebugEnabled()` is this really necessary? I'd suggest use parameterized logging rather than add if a if-clausel. http://www.slf4j.org/faq.html#logging_performance
core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
<https://reviews.apache.org/r/27085/#comment99195>
For better code understanding, maybe better group `updateConnectorStatement` `deleteInput` and `deleteConfig` in three blocks?
deleteInput = conn.prepareStatement(STMT_DELETE_INPUTS_FOR_CONFIGURABLE);
deleteInput.setLong(1, mConnector.getPersistenceId());
deleteInput.executeUpdate();
deleteConfig = conn.prepareStatement(STMT_DELETE_CONFIGS_FOR_CONFIGURABLE);
deleteConfig.setLong(1, mConnector.getPersistenceId());
deleteConfig.executeUpdate();
core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
<https://reviews.apache.org/r/27085/#comment99196>
Can we use `closeStatements()` as other classes do?
core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
<https://reviews.apache.org/r/27085/#comment99198>
Nice to have:
`return submission != null && submission.getStatus().isRunning();`
core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
<https://reviews.apache.org/r/27085/#comment99199>
How about create local `ResultSet` (and close it locally) instead creating one `ResultSet` out of the for-block?
please search for similar occurences
core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
<https://reviews.apache.org/r/27085/#comment99202>
no close resultset? (please search for similar occurences)
core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
<https://reviews.apache.org/r/27085/#comment99206>
General question, if check null is required, then please add the check to all code occurences, otherwise please remove unused null check.
core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
<https://reviews.apache.org/r/27085/#comment99208>
General question: please make methods static if it is possible.
- Qian Xu
On Oct. 24, 2014, 1:13 a.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27085/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2014, 1:13 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-158
> https://issues.apache.org/jira/browse/SQOOP-158
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit 370f476b1834c390993b31007b0fc7e1a184b891
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date: Wed Oct 22 18:30:28 2014 -0700
>
> SQOOP-1589: Sqoop2: Create common constants, error codes, and queries
>
> :100644 100644 7d78826... e41cd6c... M core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
> :000000 100644 0000000... a7ebd50... A core/src/main/java/org/apache/sqoop/repository/JdbcRepositorySchemaConstants.java
> :000000 100644 0000000... eb05567... A core/src/main/java/org/apache/sqoop/repository/JdbcRepositorySchemaQuery.java
> :100644 100644 f684e85... 097a185... M core/src/main/java/org/apache/sqoop/repository/RepositoryError.java
> :100644 100644 aad219e... 769544b... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java
> :100644 100644 633e9df... 6764b14... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
> :100644 100644 ce1830e... ff8962c... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java
> :100644 100644 85140d5... aab1e95... M repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java
> :100644 100644 37343d3... 9bcf3c8... M repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 7d78826
> core/src/main/java/org/apache/sqoop/repository/JdbcRepositorySchemaConstants.java PRE-CREATION
> core/src/main/java/org/apache/sqoop/repository/JdbcRepositorySchemaQuery.java PRE-CREATION
> core/src/main/java/org/apache/sqoop/repository/RepositoryError.java f684e85
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java aad219e
> 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/DerbySchemaQuery.java ce1830e
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java 85140d5
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java 37343d3
>
> Diff: https://reviews.apache.org/r/27085/diff/
>
>
> Testing
> -------
>
> mvn clean verify + start sqoop server without issues.
>
>
> Thanks,
>
> Abraham Elmahrek
>
>
Re: Review Request 27085: SQOOP-1589: Sqoop2: Create common constants,
error codes, and queries
Posted by Abraham Elmahrek <ab...@cloudera.com>.
> On Oct. 31, 2014, 5:28 p.m., Gwen Shapira wrote:
> > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java, line 886
> > <https://reviews.apache.org/r/27085/diff/3/?file=735747#file735747line886>
> >
> > runQuery is underdocumented, which makes the changes challenging to review.
> >
> > Maybe add few lines on what "args" are expected and how they'll be used.
Made runQuery "protected" in CommonRepositoryHandler and added docs.
> On Oct. 31, 2014, 5:28 p.m., Gwen Shapira wrote:
> > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java, line 1009
> > <https://reviews.apache.org/r/27085/diff/3/?file=735747#file735747line1009>
> >
> > This looks fairly generic after the refactoring. Think it can be moved up the hierarchy?
I've moved 'runQuery' and 'closeStatements' as well.
- Abraham
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27085/#review59359
-----------------------------------------------------------
On Oct. 28, 2014, 9:04 p.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27085/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2014, 9:04 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1589
> https://issues.apache.org/jira/browse/SQOOP-1589
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit 07f1441c6bb1a99f8e2ef81c89d2b4752fc5d76d
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date: Wed Oct 22 18:30:28 2014 -0700
>
> SQOOP-1589: Sqoop2: Create common constants, error codes, and queries
>
> :100644 100644 f25a29f... 29de641... M pom.xml
> :100644 100644 e3345c4... 8c95c0e... M repository/pom.xml
> :000000 100644 0000000... 37378c6... A repository/repository-common/pom.xml
> :000000 100644 0000000... 6d2c926... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryError.java
> :000000 100644 0000000... 3198b0f... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
> :000000 100644 0000000... cf3df00... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java
> :000000 100644 0000000... 173dcb8... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositorySchemaConstants.java
> :100644 100644 6ad6d64... 9be96db... M repository/repository-derby/pom.xml
> :100644 100644 aad219e... 769544b... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java
> :100644 100644 7f19c28... 726d4ab... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
> :100644 100644 02b11fc... c159183... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java
> :100644 100644 85140d5... 9a64ad9... M repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java
> :100644 100644 37343d3... b96b2d7... M repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java
> :100644 100644 67baaa5... 090a0b1... M server/pom.xml
> :100644 100644 7a80710... 954fc68... M test/pom.xml
>
>
> Diffs
> -----
>
> pom.xml f25a29f
> repository/pom.xml e3345c4
> repository/repository-common/pom.xml PRE-CREATION
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryError.java PRE-CREATION
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java PRE-CREATION
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java PRE-CREATION
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositorySchemaConstants.java PRE-CREATION
> repository/repository-derby/pom.xml 6ad6d64
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java aad219e
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 514b5ac
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java c894d06
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java 85140d5
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java dabb08b
> server/pom.xml 67baaa5
> test/pom.xml 7a80710
>
> Diff: https://reviews.apache.org/r/27085/diff/
>
>
> Testing
> -------
>
> mvn clean verify + start sqoop server without issues.
>
>
> Thanks,
>
> Abraham Elmahrek
>
>
Re: Review Request 27085: SQOOP-1589: Sqoop2: Create common constants,
error codes, and queries
Posted by Gwen Shapira <gs...@cloudera.com>.
> On Oct. 31, 2014, 5:28 p.m., Gwen Shapira wrote:
> > repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java, line 22
> > <https://reviews.apache.org/r/27085/diff/3/?file=735743#file735743line22>
> >
> > Perhaps we want to separate this farther into DML (insert, update, delete) and queries (that modify nothing)?
> > I'm not sure if it will make things more or less readable, would like to hear your thoughts on this.
>
> Abraham Elmahrek wrote:
> Hmmm... perhaps. Follow up Jira?
Sure. and not high priority.
> On Oct. 31, 2014, 5:28 p.m., Gwen Shapira wrote:
> > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java, line 25
> > <https://reviews.apache.org/r/27085/diff/3/?file=735746#file735746line25>
> >
> > I'm wondering if DB specific error messages should contain text that indicate the DB.
>
> Abraham Elmahrek wrote:
> Is DERBYREPO_XXXX good enough?
Note sure. I want it to be clear in the error message so it will be easy for us, customers and support to see what caused the error. Does using DERBYREP_XXX modify the error message in the log? or just the stack?
- Gwen
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27085/#review59359
-----------------------------------------------------------
On Nov. 3, 2014, 7:29 p.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27085/
> -----------------------------------------------------------
>
> (Updated Nov. 3, 2014, 7:29 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1589
> https://issues.apache.org/jira/browse/SQOOP-1589
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit 07f1441c6bb1a99f8e2ef81c89d2b4752fc5d76d
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date: Wed Oct 22 18:30:28 2014 -0700
>
> SQOOP-1589: Sqoop2: Create common constants, error codes, and queries
>
> :100644 100644 f25a29f... 29de641... M pom.xml
> :100644 100644 e3345c4... 8c95c0e... M repository/pom.xml
> :000000 100644 0000000... 37378c6... A repository/repository-common/pom.xml
> :000000 100644 0000000... 6d2c926... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryError.java
> :000000 100644 0000000... 3198b0f... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
> :000000 100644 0000000... cf3df00... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java
> :000000 100644 0000000... 173dcb8... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositorySchemaConstants.java
> :100644 100644 6ad6d64... 9be96db... M repository/repository-derby/pom.xml
> :100644 100644 aad219e... 769544b... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java
> :100644 100644 7f19c28... 726d4ab... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
> :100644 100644 02b11fc... c159183... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java
> :100644 100644 85140d5... 9a64ad9... M repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java
> :100644 100644 37343d3... b96b2d7... M repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java
> :100644 100644 67baaa5... 090a0b1... M server/pom.xml
> :100644 100644 7a80710... 954fc68... M test/pom.xml
>
>
> Diffs
> -----
>
> pom.xml 3449a6f
> repository/pom.xml e3345c4
> repository/repository-common/pom.xml PRE-CREATION
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepoConstants.java PRE-CREATION
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryError.java PRE-CREATION
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java PRE-CREATION
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java PRE-CREATION
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositorySchemaConstants.java PRE-CREATION
> repository/repository-derby/pom.xml 6ad6d64
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoConstants.java 8fbf47f
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java aad219e
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java b324f4f
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java c894d06
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java 85140d5
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java dabb08b
> server/pom.xml be2ee9b
> test/pom.xml 956aeb7
>
> Diff: https://reviews.apache.org/r/27085/diff/
>
>
> Testing
> -------
>
> mvn clean verify + start sqoop server without issues.
>
>
> Thanks,
>
> Abraham Elmahrek
>
>
Re: Review Request 27085: SQOOP-1589: Sqoop2: Create common constants,
error codes, and queries
Posted by Abraham Elmahrek <ab...@cloudera.com>.
> On Oct. 31, 2014, 5:28 p.m., Gwen Shapira wrote:
> > repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java, lines 343-346
> > <https://reviews.apache.org/r/27085/diff/3/?file=735742#file735742line343>
> >
> > This is specific for Derby - it won't work on any other DB AFAIK.
> >
> > Lets move it out of common.
Interestingly enough, this works for PostgreSQL. It doesn't seem to work for MySQL though. Moving it out.
> On Oct. 31, 2014, 5:28 p.m., Gwen Shapira wrote:
> > repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java, line 242
> > <https://reviews.apache.org/r/27085/diff/3/?file=735742#file735742line242>
> >
> > Is this an inherited method?
> > If so, any reason we can't inherit docs?
It's a private method.
> On Oct. 31, 2014, 5:28 p.m., Gwen Shapira wrote:
> > repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java, line 1138
> > <https://reviews.apache.org/r/27085/diff/3/?file=735742#file735742line1138>
> >
> > docs?
https://issues.apache.org/jira/browse/SQOOP-1658
> On Oct. 31, 2014, 5:28 p.m., Gwen Shapira wrote:
> > repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java, line 1166
> > <https://reviews.apache.org/r/27085/diff/3/?file=735742#file735742line1166>
> >
> > docs? (can be separate jira if docs never existed)
https://issues.apache.org/jira/browse/SQOOP-1658
> On Oct. 31, 2014, 5:28 p.m., Gwen Shapira wrote:
> > repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java, line 1204
> > <https://reviews.apache.org/r/27085/diff/3/?file=735742#file735742line1204>
> >
> > inherit docs?
Private method: https://issues.apache.org/jira/browse/SQOOP-1658
> On Oct. 31, 2014, 5:28 p.m., Gwen Shapira wrote:
> > repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java, line 1236
> > <https://reviews.apache.org/r/27085/diff/3/?file=735742#file735742line1236>
> >
> > docs?
Private method: https://issues.apache.org/jira/browse/SQOOP-1658
> On Oct. 31, 2014, 5:28 p.m., Gwen Shapira wrote:
> > repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java, line 22
> > <https://reviews.apache.org/r/27085/diff/3/?file=735743#file735743line22>
> >
> > Perhaps we want to separate this farther into DML (insert, update, delete) and queries (that modify nothing)?
> > I'm not sure if it will make things more or less readable, would like to hear your thoughts on this.
Hmmm... perhaps. Follow up Jira?
> On Oct. 31, 2014, 5:28 p.m., Gwen Shapira wrote:
> > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java, line 25
> > <https://reviews.apache.org/r/27085/diff/3/?file=735746#file735746line25>
> >
> > I'm wondering if DB specific error messages should contain text that indicate the DB.
Is DERBYREPO_XXXX good enough?
> On Oct. 31, 2014, 5:28 p.m., Gwen Shapira wrote:
> > repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java, line 1293
> > <https://reviews.apache.org/r/27085/diff/3/?file=735742#file735742line1293>
> >
> > inherit docs?
Private method: https://issues.apache.org/jira/browse/SQOOP-1658
> On Oct. 31, 2014, 5:28 p.m., Gwen Shapira wrote:
> > repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java, line 1264
> > <https://reviews.apache.org/r/27085/diff/3/?file=735742#file735742line1264>
> >
> > docs?
Private method: https://issues.apache.org/jira/browse/SQOOP-1658
> On Oct. 31, 2014, 5:28 p.m., Gwen Shapira wrote:
> > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java, line 941
> > <https://reviews.apache.org/r/27085/diff/3/?file=735747#file735747line941>
> >
> > Not this patch, but I can't help commenting:
> >
> > ResultSet should have a method for getting the number of rows returned/updated. No need to count.
https://issues.apache.org/jira/browse/SQOOP-1659
On Oct. 31, 2014, 5:28 p.m., Abraham Elmahrek wrote:
> > * I'd like some consistency around where we document the methods. And I think since all those methods are inherited, the docs should go in super. We can have a separate JIRA for all the classes where documentation does not already exist.
Couldn't agree more.
- Abraham
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27085/#review59359
-----------------------------------------------------------
On Oct. 28, 2014, 9:04 p.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27085/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2014, 9:04 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1589
> https://issues.apache.org/jira/browse/SQOOP-1589
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit 07f1441c6bb1a99f8e2ef81c89d2b4752fc5d76d
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date: Wed Oct 22 18:30:28 2014 -0700
>
> SQOOP-1589: Sqoop2: Create common constants, error codes, and queries
>
> :100644 100644 f25a29f... 29de641... M pom.xml
> :100644 100644 e3345c4... 8c95c0e... M repository/pom.xml
> :000000 100644 0000000... 37378c6... A repository/repository-common/pom.xml
> :000000 100644 0000000... 6d2c926... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryError.java
> :000000 100644 0000000... 3198b0f... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
> :000000 100644 0000000... cf3df00... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java
> :000000 100644 0000000... 173dcb8... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositorySchemaConstants.java
> :100644 100644 6ad6d64... 9be96db... M repository/repository-derby/pom.xml
> :100644 100644 aad219e... 769544b... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java
> :100644 100644 7f19c28... 726d4ab... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
> :100644 100644 02b11fc... c159183... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java
> :100644 100644 85140d5... 9a64ad9... M repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java
> :100644 100644 37343d3... b96b2d7... M repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java
> :100644 100644 67baaa5... 090a0b1... M server/pom.xml
> :100644 100644 7a80710... 954fc68... M test/pom.xml
>
>
> Diffs
> -----
>
> pom.xml f25a29f
> repository/pom.xml e3345c4
> repository/repository-common/pom.xml PRE-CREATION
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryError.java PRE-CREATION
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java PRE-CREATION
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java PRE-CREATION
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositorySchemaConstants.java PRE-CREATION
> repository/repository-derby/pom.xml 6ad6d64
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java aad219e
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 514b5ac
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java c894d06
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java 85140d5
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java dabb08b
> server/pom.xml 67baaa5
> test/pom.xml 7a80710
>
> Diff: https://reviews.apache.org/r/27085/diff/
>
>
> Testing
> -------
>
> mvn clean verify + start sqoop server without issues.
>
>
> Thanks,
>
> Abraham Elmahrek
>
>
Re: Review Request 27085: SQOOP-1589: Sqoop2: Create common constants,
error codes, and queries
Posted by Gwen Shapira <gs...@cloudera.com>.
> On Oct. 31, 2014, 5:28 p.m., Gwen Shapira wrote:
> > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java, line 25
> > <https://reviews.apache.org/r/27085/diff/3/?file=735746#file735746line25>
> >
> > I'm wondering if DB specific error messages should contain text that indicate the DB.
>
> Abraham Elmahrek wrote:
> Is DERBYREPO_XXXX good enough?
>
> Gwen Shapira wrote:
> Note sure. I want it to be clear in the error message so it will be easy for us, customers and support to see what caused the error. Does using DERBYREP_XXX modify the error message in the log? or just the stack?
>
> Abraham Elmahrek wrote:
> Hmm if we add a "name" to repository. We could log that using the "logException" method.
ok, another JIRA is fine :)
- Gwen
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27085/#review59359
-----------------------------------------------------------
On Nov. 3, 2014, 7:29 p.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27085/
> -----------------------------------------------------------
>
> (Updated Nov. 3, 2014, 7:29 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1589
> https://issues.apache.org/jira/browse/SQOOP-1589
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit 07f1441c6bb1a99f8e2ef81c89d2b4752fc5d76d
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date: Wed Oct 22 18:30:28 2014 -0700
>
> SQOOP-1589: Sqoop2: Create common constants, error codes, and queries
>
> :100644 100644 f25a29f... 29de641... M pom.xml
> :100644 100644 e3345c4... 8c95c0e... M repository/pom.xml
> :000000 100644 0000000... 37378c6... A repository/repository-common/pom.xml
> :000000 100644 0000000... 6d2c926... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryError.java
> :000000 100644 0000000... 3198b0f... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
> :000000 100644 0000000... cf3df00... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java
> :000000 100644 0000000... 173dcb8... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositorySchemaConstants.java
> :100644 100644 6ad6d64... 9be96db... M repository/repository-derby/pom.xml
> :100644 100644 aad219e... 769544b... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java
> :100644 100644 7f19c28... 726d4ab... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
> :100644 100644 02b11fc... c159183... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java
> :100644 100644 85140d5... 9a64ad9... M repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java
> :100644 100644 37343d3... b96b2d7... M repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java
> :100644 100644 67baaa5... 090a0b1... M server/pom.xml
> :100644 100644 7a80710... 954fc68... M test/pom.xml
>
>
> Diffs
> -----
>
> pom.xml 3449a6f
> repository/pom.xml e3345c4
> repository/repository-common/pom.xml PRE-CREATION
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepoConstants.java PRE-CREATION
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryError.java PRE-CREATION
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java PRE-CREATION
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java PRE-CREATION
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositorySchemaConstants.java PRE-CREATION
> repository/repository-derby/pom.xml 6ad6d64
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoConstants.java 8fbf47f
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java aad219e
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java b324f4f
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java c894d06
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java 85140d5
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java dabb08b
> server/pom.xml be2ee9b
> test/pom.xml 956aeb7
>
> Diff: https://reviews.apache.org/r/27085/diff/
>
>
> Testing
> -------
>
> mvn clean verify + start sqoop server without issues.
>
>
> Thanks,
>
> Abraham Elmahrek
>
>
Re: Review Request 27085: SQOOP-1589: Sqoop2: Create common constants,
error codes, and queries
Posted by Abraham Elmahrek <ab...@cloudera.com>.
> On Oct. 31, 2014, 5:28 p.m., Gwen Shapira wrote:
> > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java, line 25
> > <https://reviews.apache.org/r/27085/diff/3/?file=735746#file735746line25>
> >
> > I'm wondering if DB specific error messages should contain text that indicate the DB.
>
> Abraham Elmahrek wrote:
> Is DERBYREPO_XXXX good enough?
>
> Gwen Shapira wrote:
> Note sure. I want it to be clear in the error message so it will be easy for us, customers and support to see what caused the error. Does using DERBYREP_XXX modify the error message in the log? or just the stack?
Hmm if we add a "name" to repository. We could log that using the "logException" method.
- Abraham
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27085/#review59359
-----------------------------------------------------------
On Nov. 3, 2014, 7:29 p.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27085/
> -----------------------------------------------------------
>
> (Updated Nov. 3, 2014, 7:29 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1589
> https://issues.apache.org/jira/browse/SQOOP-1589
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit 07f1441c6bb1a99f8e2ef81c89d2b4752fc5d76d
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date: Wed Oct 22 18:30:28 2014 -0700
>
> SQOOP-1589: Sqoop2: Create common constants, error codes, and queries
>
> :100644 100644 f25a29f... 29de641... M pom.xml
> :100644 100644 e3345c4... 8c95c0e... M repository/pom.xml
> :000000 100644 0000000... 37378c6... A repository/repository-common/pom.xml
> :000000 100644 0000000... 6d2c926... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryError.java
> :000000 100644 0000000... 3198b0f... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
> :000000 100644 0000000... cf3df00... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java
> :000000 100644 0000000... 173dcb8... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositorySchemaConstants.java
> :100644 100644 6ad6d64... 9be96db... M repository/repository-derby/pom.xml
> :100644 100644 aad219e... 769544b... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java
> :100644 100644 7f19c28... 726d4ab... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
> :100644 100644 02b11fc... c159183... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java
> :100644 100644 85140d5... 9a64ad9... M repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java
> :100644 100644 37343d3... b96b2d7... M repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java
> :100644 100644 67baaa5... 090a0b1... M server/pom.xml
> :100644 100644 7a80710... 954fc68... M test/pom.xml
>
>
> Diffs
> -----
>
> pom.xml 3449a6f
> repository/pom.xml e3345c4
> repository/repository-common/pom.xml PRE-CREATION
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepoConstants.java PRE-CREATION
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryError.java PRE-CREATION
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java PRE-CREATION
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java PRE-CREATION
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositorySchemaConstants.java PRE-CREATION
> repository/repository-derby/pom.xml 6ad6d64
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoConstants.java 8fbf47f
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java aad219e
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java b324f4f
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java c894d06
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java 85140d5
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java dabb08b
> server/pom.xml be2ee9b
> test/pom.xml 956aeb7
>
> Diff: https://reviews.apache.org/r/27085/diff/
>
>
> Testing
> -------
>
> mvn clean verify + start sqoop server without issues.
>
>
> Thanks,
>
> Abraham Elmahrek
>
>
Re: Review Request 27085: SQOOP-1589: Sqoop2: Create common constants,
error codes, and queries
Posted by Gwen Shapira <gs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27085/#review59359
-----------------------------------------------------------
Awesome stuff. I think it will make life a lot better down the road. It also makes future refactoring easier since most logic is in one place now.
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
<https://reviews.apache.org/r/27085/#comment100627>
Is this an inherited method?
If so, any reason we can't inherit docs?
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
<https://reviews.apache.org/r/27085/#comment100625>
This is specific for Derby - it won't work on any other DB AFAIK.
Lets move it out of common.
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
<https://reviews.apache.org/r/27085/#comment100628>
docs?
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
<https://reviews.apache.org/r/27085/#comment100629>
docs? (can be separate jira if docs never existed)
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
<https://reviews.apache.org/r/27085/#comment100630>
inherit docs?
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
<https://reviews.apache.org/r/27085/#comment100631>
docs?
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
<https://reviews.apache.org/r/27085/#comment100632>
docs?
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
<https://reviews.apache.org/r/27085/#comment100633>
inherit docs?
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java
<https://reviews.apache.org/r/27085/#comment100634>
Perhaps we want to separate this farther into DML (insert, update, delete) and queries (that modify nothing)?
I'm not sure if it will make things more or less readable, would like to hear your thoughts on this.
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java
<https://reviews.apache.org/r/27085/#comment100636>
I'm wondering if DB specific error messages should contain text that indicate the DB.
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
<https://reviews.apache.org/r/27085/#comment100637>
runQuery is underdocumented, which makes the changes challenging to review.
Maybe add few lines on what "args" are expected and how they'll be used.
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
<https://reviews.apache.org/r/27085/#comment100638>
Not this patch, but I can't help commenting:
ResultSet should have a method for getting the number of rows returned/updated. No need to count.
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
<https://reviews.apache.org/r/27085/#comment100639>
This looks fairly generic after the refactoring. Think it can be moved up the hierarchy?
* I'd like some consistency around where we document the methods. And I think since all those methods are inherited, the docs should go in super. We can have a separate JIRA for all the classes where documentation does not already exist.
- Gwen Shapira
On Oct. 28, 2014, 9:04 p.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27085/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2014, 9:04 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1589
> https://issues.apache.org/jira/browse/SQOOP-1589
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit 07f1441c6bb1a99f8e2ef81c89d2b4752fc5d76d
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date: Wed Oct 22 18:30:28 2014 -0700
>
> SQOOP-1589: Sqoop2: Create common constants, error codes, and queries
>
> :100644 100644 f25a29f... 29de641... M pom.xml
> :100644 100644 e3345c4... 8c95c0e... M repository/pom.xml
> :000000 100644 0000000... 37378c6... A repository/repository-common/pom.xml
> :000000 100644 0000000... 6d2c926... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryError.java
> :000000 100644 0000000... 3198b0f... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
> :000000 100644 0000000... cf3df00... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java
> :000000 100644 0000000... 173dcb8... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositorySchemaConstants.java
> :100644 100644 6ad6d64... 9be96db... M repository/repository-derby/pom.xml
> :100644 100644 aad219e... 769544b... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java
> :100644 100644 7f19c28... 726d4ab... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
> :100644 100644 02b11fc... c159183... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java
> :100644 100644 85140d5... 9a64ad9... M repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java
> :100644 100644 37343d3... b96b2d7... M repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java
> :100644 100644 67baaa5... 090a0b1... M server/pom.xml
> :100644 100644 7a80710... 954fc68... M test/pom.xml
>
>
> Diffs
> -----
>
> pom.xml f25a29f
> repository/pom.xml e3345c4
> repository/repository-common/pom.xml PRE-CREATION
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryError.java PRE-CREATION
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java PRE-CREATION
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java PRE-CREATION
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositorySchemaConstants.java PRE-CREATION
> repository/repository-derby/pom.xml 6ad6d64
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java aad219e
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 514b5ac
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java c894d06
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java 85140d5
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java dabb08b
> server/pom.xml 67baaa5
> test/pom.xml 7a80710
>
> Diff: https://reviews.apache.org/r/27085/diff/
>
>
> Testing
> -------
>
> mvn clean verify + start sqoop server without issues.
>
>
> Thanks,
>
> Abraham Elmahrek
>
>
Re: Review Request 27085: SQOOP-1589: Sqoop2: Create common constants,
error codes, and queries
Posted by Gwen Shapira <gs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27085/#review59623
-----------------------------------------------------------
Ship it!
Ship It!
- Gwen Shapira
On Nov. 3, 2014, 7:29 p.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27085/
> -----------------------------------------------------------
>
> (Updated Nov. 3, 2014, 7:29 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1589
> https://issues.apache.org/jira/browse/SQOOP-1589
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit 07f1441c6bb1a99f8e2ef81c89d2b4752fc5d76d
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date: Wed Oct 22 18:30:28 2014 -0700
>
> SQOOP-1589: Sqoop2: Create common constants, error codes, and queries
>
> :100644 100644 f25a29f... 29de641... M pom.xml
> :100644 100644 e3345c4... 8c95c0e... M repository/pom.xml
> :000000 100644 0000000... 37378c6... A repository/repository-common/pom.xml
> :000000 100644 0000000... 6d2c926... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryError.java
> :000000 100644 0000000... 3198b0f... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
> :000000 100644 0000000... cf3df00... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java
> :000000 100644 0000000... 173dcb8... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositorySchemaConstants.java
> :100644 100644 6ad6d64... 9be96db... M repository/repository-derby/pom.xml
> :100644 100644 aad219e... 769544b... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java
> :100644 100644 7f19c28... 726d4ab... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
> :100644 100644 02b11fc... c159183... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java
> :100644 100644 85140d5... 9a64ad9... M repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java
> :100644 100644 37343d3... b96b2d7... M repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java
> :100644 100644 67baaa5... 090a0b1... M server/pom.xml
> :100644 100644 7a80710... 954fc68... M test/pom.xml
>
>
> Diffs
> -----
>
> pom.xml 3449a6f
> repository/pom.xml e3345c4
> repository/repository-common/pom.xml PRE-CREATION
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepoConstants.java PRE-CREATION
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryError.java PRE-CREATION
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java PRE-CREATION
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java PRE-CREATION
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositorySchemaConstants.java PRE-CREATION
> repository/repository-derby/pom.xml 6ad6d64
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoConstants.java 8fbf47f
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java aad219e
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java b324f4f
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java c894d06
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java 85140d5
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java dabb08b
> server/pom.xml be2ee9b
> test/pom.xml 956aeb7
>
> Diff: https://reviews.apache.org/r/27085/diff/
>
>
> Testing
> -------
>
> mvn clean verify + start sqoop server without issues.
>
>
> Thanks,
>
> Abraham Elmahrek
>
>
Re: Review Request 27085: SQOOP-1589: Sqoop2: Create common constants,
error codes, and queries
Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27085/
-----------------------------------------------------------
(Updated Nov. 3, 2014, 7:29 p.m.)
Review request for Sqoop.
Bugs: SQOOP-1589
https://issues.apache.org/jira/browse/SQOOP-1589
Repository: sqoop-sqoop2
Description
-------
commit 07f1441c6bb1a99f8e2ef81c89d2b4752fc5d76d
Author: Abraham Elmahrek <ab...@elmahrek.com>
Date: Wed Oct 22 18:30:28 2014 -0700
SQOOP-1589: Sqoop2: Create common constants, error codes, and queries
:100644 100644 f25a29f... 29de641... M pom.xml
:100644 100644 e3345c4... 8c95c0e... M repository/pom.xml
:000000 100644 0000000... 37378c6... A repository/repository-common/pom.xml
:000000 100644 0000000... 6d2c926... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryError.java
:000000 100644 0000000... 3198b0f... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
:000000 100644 0000000... cf3df00... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java
:000000 100644 0000000... 173dcb8... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositorySchemaConstants.java
:100644 100644 6ad6d64... 9be96db... M repository/repository-derby/pom.xml
:100644 100644 aad219e... 769544b... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java
:100644 100644 7f19c28... 726d4ab... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
:100644 100644 02b11fc... c159183... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java
:100644 100644 85140d5... 9a64ad9... M repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java
:100644 100644 37343d3... b96b2d7... M repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java
:100644 100644 67baaa5... 090a0b1... M server/pom.xml
:100644 100644 7a80710... 954fc68... M test/pom.xml
Diffs (updated)
-----
pom.xml 3449a6f
repository/pom.xml e3345c4
repository/repository-common/pom.xml PRE-CREATION
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepoConstants.java PRE-CREATION
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryError.java PRE-CREATION
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java PRE-CREATION
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java PRE-CREATION
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositorySchemaConstants.java PRE-CREATION
repository/repository-derby/pom.xml 6ad6d64
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoConstants.java 8fbf47f
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java aad219e
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java b324f4f
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java c894d06
repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java 85140d5
repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java dabb08b
server/pom.xml be2ee9b
test/pom.xml 956aeb7
Diff: https://reviews.apache.org/r/27085/diff/
Testing
-------
mvn clean verify + start sqoop server without issues.
Thanks,
Abraham Elmahrek
Re: Review Request 27085: SQOOP-1589: Sqoop2: Create common constants,
error codes, and queries
Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27085/
-----------------------------------------------------------
(Updated Oct. 28, 2014, 9:04 p.m.)
Review request for Sqoop.
Changes
-------
Rebase + update error codes + add @Override and "Inherit docs".
Bugs: SQOOP-1589
https://issues.apache.org/jira/browse/SQOOP-1589
Repository: sqoop-sqoop2
Description
-------
commit 07f1441c6bb1a99f8e2ef81c89d2b4752fc5d76d
Author: Abraham Elmahrek <ab...@elmahrek.com>
Date: Wed Oct 22 18:30:28 2014 -0700
SQOOP-1589: Sqoop2: Create common constants, error codes, and queries
:100644 100644 f25a29f... 29de641... M pom.xml
:100644 100644 e3345c4... 8c95c0e... M repository/pom.xml
:000000 100644 0000000... 37378c6... A repository/repository-common/pom.xml
:000000 100644 0000000... 6d2c926... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryError.java
:000000 100644 0000000... 3198b0f... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
:000000 100644 0000000... cf3df00... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java
:000000 100644 0000000... 173dcb8... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositorySchemaConstants.java
:100644 100644 6ad6d64... 9be96db... M repository/repository-derby/pom.xml
:100644 100644 aad219e... 769544b... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java
:100644 100644 7f19c28... 726d4ab... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
:100644 100644 02b11fc... c159183... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java
:100644 100644 85140d5... 9a64ad9... M repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java
:100644 100644 37343d3... b96b2d7... M repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java
:100644 100644 67baaa5... 090a0b1... M server/pom.xml
:100644 100644 7a80710... 954fc68... M test/pom.xml
Diffs (updated)
-----
pom.xml f25a29f
repository/pom.xml e3345c4
repository/repository-common/pom.xml PRE-CREATION
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryError.java PRE-CREATION
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java PRE-CREATION
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java PRE-CREATION
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositorySchemaConstants.java PRE-CREATION
repository/repository-derby/pom.xml 6ad6d64
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java aad219e
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 514b5ac
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java c894d06
repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java 85140d5
repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java dabb08b
server/pom.xml 67baaa5
test/pom.xml 7a80710
Diff: https://reviews.apache.org/r/27085/diff/
Testing
-------
mvn clean verify + start sqoop server without issues.
Thanks,
Abraham Elmahrek
Re: Review Request 27085: SQOOP-1589: Sqoop2: Create common constants,
error codes, and queries
Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27085/
-----------------------------------------------------------
(Updated Oct. 28, 2014, 6:26 p.m.)
Review request for Sqoop.
Changes
-------
I've had some thoughts and decided to pull the common code into a separate package.
Bugs: SQOOP-1589
https://issues.apache.org/jira/browse/SQOOP-1589
Repository: sqoop-sqoop2
Description (updated)
-------
commit 07f1441c6bb1a99f8e2ef81c89d2b4752fc5d76d
Author: Abraham Elmahrek <ab...@elmahrek.com>
Date: Wed Oct 22 18:30:28 2014 -0700
SQOOP-1589: Sqoop2: Create common constants, error codes, and queries
:100644 100644 f25a29f... 29de641... M pom.xml
:100644 100644 e3345c4... 8c95c0e... M repository/pom.xml
:000000 100644 0000000... 37378c6... A repository/repository-common/pom.xml
:000000 100644 0000000... 6d2c926... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryError.java
:000000 100644 0000000... 3198b0f... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
:000000 100644 0000000... cf3df00... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java
:000000 100644 0000000... 173dcb8... A repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositorySchemaConstants.java
:100644 100644 6ad6d64... 9be96db... M repository/repository-derby/pom.xml
:100644 100644 aad219e... 769544b... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java
:100644 100644 7f19c28... 726d4ab... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
:100644 100644 02b11fc... c159183... M repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java
:100644 100644 85140d5... 9a64ad9... M repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java
:100644 100644 37343d3... b96b2d7... M repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java
:100644 100644 67baaa5... 090a0b1... M server/pom.xml
:100644 100644 7a80710... 954fc68... M test/pom.xml
Diffs (updated)
-----
pom.xml f25a29f
repository/pom.xml e3345c4
repository/repository-common/pom.xml PRE-CREATION
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryError.java PRE-CREATION
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java PRE-CREATION
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java PRE-CREATION
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositorySchemaConstants.java PRE-CREATION
repository/repository-derby/pom.xml 6ad6d64
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java aad219e
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 7f19c28
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java 02b11fc
repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java 85140d5
repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java 37343d3
server/pom.xml 67baaa5
test/pom.xml 7a80710
Diff: https://reviews.apache.org/r/27085/diff/
Testing
-------
mvn clean verify + start sqoop server without issues.
Thanks,
Abraham Elmahrek