You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Jarek Cecho <ja...@apache.org> on 2014/10/10 04:57:10 UTC
Review Request 26539: SQOOP-1575 Sqoop2: Validations: Migrate HDFS
connector to new validators
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26539/
-----------------------------------------------------------
Review request for Sqoop.
Bugs: SQOOP-1575
https://issues.apache.org/jira/browse/SQOOP-1575
Repository: sqoop-SQOOP-1367
Description
-------
I've provided new validators into HDFS connector configuration objects. I'm pretty much re-implementing what is available in the old validator.
Diffs
-----
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/FromJobConfig.java 037fe59
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/ToJobConfig.java 2dfd738
Diff: https://reviews.apache.org/r/26539/diff/
Testing
-------
Tested on real cluster, seems to be working just fine.
Thanks,
Jarek Cecho
Re: Review Request 26539: SQOOP-1575 Sqoop2: Validations: Migrate HDFS
connector to new validators
Posted by Jarek Cecho <ja...@apache.org>.
> On Oct. 10, 2014, 9:51 p.m., Gwen Shapira wrote:
> > The patch looks good.
> >
> > I'm just confused about the location of the package. Looks like the old validators were part of the spi and the new are in common. Why did we move them? I thought they made a lot of sense in the spi.
Very good question. The original contract of SPI was list of classes that connector developer should extent in their code. So for example even the old Validator was kind of split between the SPI and common - only one class has been in SPI whereas the others were in common. Sadly with the new Validators I can't longer put one class that is suppose to be extended into SPI and rest into common as I would end up with maven cyclic dependency (SPI depends on common, common depends on SPI). Hence I've took the easiest path and put all code into common. I'm open to revisit that if we want to do it differently. Perhaps putting some examples to connector SDK instead of common, ... It seems as a orthogonal to this JIRA though?
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26539/#review56231
-----------------------------------------------------------
On Oct. 10, 2014, 2:57 a.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26539/
> -----------------------------------------------------------
>
> (Updated Oct. 10, 2014, 2:57 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1575
> https://issues.apache.org/jira/browse/SQOOP-1575
>
>
> Repository: sqoop-SQOOP-1367
>
>
> Description
> -------
>
> I've provided new validators into HDFS connector configuration objects. I'm pretty much re-implementing what is available in the old validator.
>
>
> Diffs
> -----
>
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/FromJobConfig.java 037fe59
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/ToJobConfig.java 2dfd738
>
> Diff: https://reviews.apache.org/r/26539/diff/
>
>
> Testing
> -------
>
> Tested on real cluster, seems to be working just fine.
>
>
> Thanks,
>
> Jarek Cecho
>
>
Re: Review Request 26539: SQOOP-1575 Sqoop2: Validations: Migrate HDFS
connector to new validators
Posted by Gwen Shapira <gs...@cloudera.com>.
> On Oct. 10, 2014, 9:51 p.m., Gwen Shapira wrote:
> > The patch looks good.
> >
> > I'm just confused about the location of the package. Looks like the old validators were part of the spi and the new are in common. Why did we move them? I thought they made a lot of sense in the spi.
>
> Jarek Cecho wrote:
> Very good question. The original contract of SPI was list of classes that connector developer should extent in their code. So for example even the old Validator was kind of split between the SPI and common - only one class has been in SPI whereas the others were in common. Sadly with the new Validators I can't longer put one class that is suppose to be extended into SPI and rest into common as I would end up with maven cyclic dependency (SPI depends on common, common depends on SPI). Hence I've took the easiest path and put all code into common. I'm open to revisit that if we want to do it differently. Perhaps putting some examples to connector SDK instead of common, ... It seems as a orthogonal to this JIRA though?
Totally orthogonal. Was just taking the opportunity to ask about the new validator design.
- Gwen
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26539/#review56231
-----------------------------------------------------------
On Oct. 10, 2014, 2:57 a.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26539/
> -----------------------------------------------------------
>
> (Updated Oct. 10, 2014, 2:57 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1575
> https://issues.apache.org/jira/browse/SQOOP-1575
>
>
> Repository: sqoop-SQOOP-1367
>
>
> Description
> -------
>
> I've provided new validators into HDFS connector configuration objects. I'm pretty much re-implementing what is available in the old validator.
>
>
> Diffs
> -----
>
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/FromJobConfig.java 037fe59
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/ToJobConfig.java 2dfd738
>
> Diff: https://reviews.apache.org/r/26539/diff/
>
>
> Testing
> -------
>
> Tested on real cluster, seems to be working just fine.
>
>
> Thanks,
>
> Jarek Cecho
>
>
Re: Review Request 26539: SQOOP-1575 Sqoop2: Validations: Migrate HDFS
connector to new validators
Posted by Jarek Cecho <ja...@apache.org>.
> On Oct. 10, 2014, 9:51 p.m., Gwen Shapira wrote:
> > The patch looks good.
> >
> > I'm just confused about the location of the package. Looks like the old validators were part of the spi and the new are in common. Why did we move them? I thought they made a lot of sense in the spi.
>
> Jarek Cecho wrote:
> Very good question. The original contract of SPI was list of classes that connector developer should extent in their code. So for example even the old Validator was kind of split between the SPI and common - only one class has been in SPI whereas the others were in common. Sadly with the new Validators I can't longer put one class that is suppose to be extended into SPI and rest into common as I would end up with maven cyclic dependency (SPI depends on common, common depends on SPI). Hence I've took the easiest path and put all code into common. I'm open to revisit that if we want to do it differently. Perhaps putting some examples to connector SDK instead of common, ... It seems as a orthogonal to this JIRA though?
>
> Gwen Shapira wrote:
> Totally orthogonal. Was just taking the opportunity to ask about the new validator design.
Feel free to open a subtask under the umbrella JIRA SQOOP-1439 to discuss this one further. I'm really open to improve that :)
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26539/#review56231
-----------------------------------------------------------
On Oct. 10, 2014, 2:57 a.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26539/
> -----------------------------------------------------------
>
> (Updated Oct. 10, 2014, 2:57 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1575
> https://issues.apache.org/jira/browse/SQOOP-1575
>
>
> Repository: sqoop-SQOOP-1367
>
>
> Description
> -------
>
> I've provided new validators into HDFS connector configuration objects. I'm pretty much re-implementing what is available in the old validator.
>
>
> Diffs
> -----
>
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/FromJobConfig.java 037fe59
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/ToJobConfig.java 2dfd738
>
> Diff: https://reviews.apache.org/r/26539/diff/
>
>
> Testing
> -------
>
> Tested on real cluster, seems to be working just fine.
>
>
> Thanks,
>
> Jarek Cecho
>
>
Re: Review Request 26539: SQOOP-1575 Sqoop2: Validations: Migrate HDFS
connector to new validators
Posted by Gwen Shapira <gs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26539/#review56231
-----------------------------------------------------------
The patch looks good.
I'm just confused about the location of the package. Looks like the old validators were part of the spi and the new are in common. Why did we move them? I thought they made a lot of sense in the spi.
- Gwen Shapira
On Oct. 10, 2014, 2:57 a.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26539/
> -----------------------------------------------------------
>
> (Updated Oct. 10, 2014, 2:57 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1575
> https://issues.apache.org/jira/browse/SQOOP-1575
>
>
> Repository: sqoop-SQOOP-1367
>
>
> Description
> -------
>
> I've provided new validators into HDFS connector configuration objects. I'm pretty much re-implementing what is available in the old validator.
>
>
> Diffs
> -----
>
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/FromJobConfig.java 037fe59
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/ToJobConfig.java 2dfd738
>
> Diff: https://reviews.apache.org/r/26539/diff/
>
>
> Testing
> -------
>
> Tested on real cluster, seems to be working just fine.
>
>
> Thanks,
>
> Jarek Cecho
>
>
Re: Review Request 26539: SQOOP-1575 Sqoop2: Validations: Migrate HDFS
connector to new validators
Posted by Gwen Shapira <gs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26539/#review56256
-----------------------------------------------------------
Ship it!
Ship It!
- Gwen Shapira
On Oct. 10, 2014, 2:57 a.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26539/
> -----------------------------------------------------------
>
> (Updated Oct. 10, 2014, 2:57 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1575
> https://issues.apache.org/jira/browse/SQOOP-1575
>
>
> Repository: sqoop-SQOOP-1367
>
>
> Description
> -------
>
> I've provided new validators into HDFS connector configuration objects. I'm pretty much re-implementing what is available in the old validator.
>
>
> Diffs
> -----
>
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/FromJobConfig.java 037fe59
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/ToJobConfig.java 2dfd738
>
> Diff: https://reviews.apache.org/r/26539/diff/
>
>
> Testing
> -------
>
> Tested on real cluster, seems to be working just fine.
>
>
> Thanks,
>
> Jarek Cecho
>
>