Re: Review Request 28828: IDF API changes

2014-12-10 Thread Veena Basavaraj
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28828/ --- (Updated Dec. 10, 2014, 8:56 a.m.) Review request for Sqoop. Bugs: SQOOP-1811

Re: Review Request 28828: IDF API changes

2014-12-10 Thread Veena Basavaraj
> On Dec. 9, 2014, 6:54 p.m., Jarek Cecho wrote: > > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/IntermediateDataFormat.java, > > line 119 > > > > > > Nit: Whitespace. i have uploaded the pat

Re: Review Request 28828: IDF API changes

2014-12-09 Thread Jarek Cecho
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28828/#review64499 --- Ship it! I believe that I have only one nit, otherwise it looks goo

Re: Review Request 28828: IDF API changes

2014-12-09 Thread Jarek Cecho
> On Dec. 9, 2014, 2:48 p.m., Jarek Cecho wrote: > > spi/src/main/java/org/apache/sqoop/connector/spi/SqoopConnector.java, line > > 95 > > > > > > I'm wondering why this change? It seems reasonable to have default > >

Re: Review Request 28828: IDF API changes

2014-12-09 Thread Veena Basavaraj
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28828/ --- (Updated Dec. 9, 2014, 9:41 a.m.) Review request for Sqoop. Bugs: SQOOP-1811

Re: Review Request 28828: IDF API changes

2014-12-09 Thread Veena Basavaraj
> On Dec. 9, 2014, 6:48 a.m., Jarek Cecho wrote: > > spi/src/main/java/org/apache/sqoop/connector/spi/SqoopConnector.java, line > > 95 > > > > > > I'm wondering why this change? It seems reasonable to have default > >

Re: Review Request 28828: IDF API changes

2014-12-09 Thread Veena Basavaraj
> On Dec. 9, 2014, 12:04 a.m., Veena Basavaraj wrote: > > spi/src/main/java/org/apache/sqoop/idf/IntermediateDataFormat.java, line 101 > > > > > > Jarcec: should we also be consistent and rename it to sqoopObjectData()

Re: Review Request 28828: IDF API changes

2014-12-09 Thread Veena Basavaraj
> On Dec. 9, 2014, 6:48 a.m., Jarek Cecho wrote: > > execution/mapreduce/pom.xml, lines 36-39 > > > > > > Wondering why is execution engine depending on connector-sdk? > > Veena Basavaraj wrote: > since tests use C

Re: Review Request 28828: IDF API changes

2014-12-09 Thread Veena Basavaraj
> On Dec. 9, 2014, 6:48 a.m., Jarek Cecho wrote: > > core/pom.xml, lines 42-44 > > > > > > Wondering why is core depending on joda-time directly? > > Veena Basavaraj wrote: > jobManaager. > > Jarek Cecho wrote: >

Re: Review Request 28828: IDF API changes

2014-12-09 Thread Veena Basavaraj
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28828/#review64383 --- core/pom.xml

Re: Review Request 28828: IDF API changes

2014-12-09 Thread Jarek Cecho
> On Dec. 9, 2014, 2:48 p.m., Jarek Cecho wrote: > > core/pom.xml, lines 42-44 > > > > > > Wondering why is core depending on joda-time directly? > > Veena Basavaraj wrote: > jobManaager. I see, the JobManager is

Re: Review Request 28828: IDF API changes

2014-12-09 Thread Veena Basavaraj
> On Dec. 9, 2014, 6:48 a.m., Jarek Cecho wrote: > > core/pom.xml, lines 42-44 > > > > > > Wondering why is core depending on joda-time directly? jobManaager. > On Dec. 9, 2014, 6:48 a.m., Jarek Cecho wrote: > > core

Re: Review Request 28828: IDF API changes

2014-12-09 Thread Jarek Cecho
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28828/#review64374 --- Overall it looks good, I do have several questions though: connect

Re: Review Request 28828: IDF API changes

2014-12-09 Thread Veena Basavaraj
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28828/ --- (Updated Dec. 9, 2014, 12:17 a.m.) Review request for Sqoop. Bugs: SQOOP-1811

Re: Review Request 28828: IDF API changes

2014-12-09 Thread Veena Basavaraj
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28828/#review64349 --- spi/src/main/java/org/apache/sqoop/idf/IntermediateDataFormat.java

Re: Review Request 28828: IDF API changes

2014-12-08 Thread Veena Basavaraj
> On Dec. 8, 2014, 6:06 p.m., Abraham Elmahrek wrote: > > spi/src/main/java/org/apache/sqoop/idf/IntermediateDataFormat.java, line 53 > > > > > > This is not seen by connector developers. It does not belong in the SPI.

Re: Review Request 28828: IDF API changes

2014-12-08 Thread Abraham Elmahrek
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28828/#review64329 --- connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/con

Re: Review Request 28828: IDF API changes

2014-12-08 Thread Veena Basavaraj
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28828/ --- (Updated Dec. 8, 2014, 3:13 p.m.) Review request for Sqoop. Bugs: SQOOP-1811