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/11/03 18:57:06 UTC
Re: Review Request 27085: SQOOP-1589: Sqoop2: Create common constants,
error codes, and queries
> 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
>
>