-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9479/
-----------------------------------------------------------

Review request for Sqoop.
Description
-------

I've improved the Framework validator as suggested.
This addresses bug SQOOP-892.
    https://issues.apache.org/jira/browse/SQOOP-892
Diffs
-----

  core/src/main/java/org/apache/sqoop/framework/FrameworkValidator.java 6f9a6fca7d95de7aa098c2e094fd98bb744a49b6
  core/src/main/java/org/apache/sqoop/framework/configuration/ExportJobConfiguration.java d533089c4602fd7740502b6d1607ba0716acf6bb
  core/src/test/java/org/apache/sqoop/framework/TestFrameworkValidator.java PRE-CREATION

Diff: https://reviews.apache.org/r/9479/diff/
Testing
-------

I've included new unit test + tested on real cluster.
Thanks,

Jarek Cecho

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9479/#review16958
-----------------------------------------------------------
Looks good. I have a minor comment regarding the error messages.
core/src/main/java/org/apache/sqoop/framework/FrameworkValidator.java
<https://reviews.apache.org/r/9479/#comment35935>

    Can you change the message to something like "The throttling threshold must be greater than 1"?

core/src/main/java/org/apache/sqoop/framework/FrameworkValidator.java
<https://reviews.apache.org/r/9479/#comment35936>

    Same as above.
- Cheolsoo Park
On Feb. 15, 2013, 11:41 p.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9479/
> -----------------------------------------------------------
>
> (Updated Feb. 15, 2013, 11:41 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> I've improved the Framework validator as suggested.
>
>
> This addresses bug SQOOP-892.
>     https://issues.apache.org/jira/browse/SQOOP-892
>
>
> Diffs
> -----
>
>   core/src/main/java/org/apache/sqoop/framework/FrameworkValidator.java 6f9a6fca7d95de7aa098c2e094fd98bb744a49b6
>   core/src/main/java/org/apache/sqoop/framework/configuration/ExportJobConfiguration.java d533089c4602fd7740502b6d1607ba0716acf6bb
>   core/src/test/java/org/apache/sqoop/framework/TestFrameworkValidator.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/9479/diff/
>
>
> Testing
> -------
>
> I've included new unit test + tested on real cluster.
>
>
> Thanks,
>
> Jarek Cecho
>
>

> On Feb. 22, 2013, 7:58 p.m., Cheolsoo Park wrote:
> > core/src/main/java/org/apache/sqoop/framework/FrameworkValidator.java, line 91
> > <https://reviews.apache.org/r/9479/diff/1/?file=259326#file259326line91>
> >
> >     Can you change the message to something like "The throttling threshold must be greater than 1"?

I've tried both this solution and the suggested one from the user perspective and here is the way the validation error will be presented to the end user:

{code}
Throttling resources

Error message: You need to specify more than one extractor
Extractors: -1
{code}

{code}
Throttling resources

Error message: The throttling threshold must be greater than 1
Extractors: -1
{code}

The original error message seems more user friendly, what do you think Cheolsoo?
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9479/#review16958
-----------------------------------------------------------
On Feb. 15, 2013, 11:41 p.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9479/
> -----------------------------------------------------------
>
> (Updated Feb. 15, 2013, 11:41 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> I've improved the Framework validator as suggested.
>
>
> This addresses bug SQOOP-892.
>     https://issues.apache.org/jira/browse/SQOOP-892
>
>
> Diffs
> -----
>
>   core/src/main/java/org/apache/sqoop/framework/FrameworkValidator.java 6f9a6fca7d95de7aa098c2e094fd98bb744a49b6
>   core/src/main/java/org/apache/sqoop/framework/configuration/ExportJobConfiguration.java d533089c4602fd7740502b6d1607ba0716acf6bb
>   core/src/test/java/org/apache/sqoop/framework/TestFrameworkValidator.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/9479/diff/
>
>
> Testing
> -------
>
> I've included new unit test + tested on real cluster.
>
>
> Thanks,
>
> Jarek Cecho
>
>

> On Feb. 22, 2013, 7:58 p.m., Cheolsoo Park wrote:
> > core/src/main/java/org/apache/sqoop/framework/FrameworkValidator.java, line 91
> > <https://reviews.apache.org/r/9479/diff/1/?file=259326#file259326line91>
> >
> >     Can you change the message to something like "The throttling threshold must be greater than 1"?
>
> Jarek Cecho wrote:
>     I've tried both this solution and the suggested one from the user perspective and here is the way the validation error will be presented to the end user:
>    
>     {code}
>     Throttling resources
>    
>     Error message: You need to specify more than one extractor
>     Extractors: -1
>     {code}
>    
>     {code}
>     Throttling resources
>    
>     Error message: The throttling threshold must be greater than 1
>     Extractors: -1
>     {code}
>    
>     The original error message seems more user friendly, what do you think Cheolsoo?

Thank you Jarcec for the clarification. I agree with you!
- Cheolsoo
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9479/#review16958
-----------------------------------------------------------
On Feb. 15, 2013, 11:41 p.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9479/
> -----------------------------------------------------------
>
> (Updated Feb. 15, 2013, 11:41 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> I've improved the Framework validator as suggested.
>
>
> This addresses bug SQOOP-892.
>     https://issues.apache.org/jira/browse/SQOOP-892
>
>
> Diffs
> -----
>
>   core/src/main/java/org/apache/sqoop/framework/FrameworkValidator.java 6f9a6fca7d95de7aa098c2e094fd98bb744a49b6
>   core/src/main/java/org/apache/sqoop/framework/configuration/ExportJobConfiguration.java d533089c4602fd7740502b6d1607ba0716acf6bb
>   core/src/test/java/org/apache/sqoop/framework/TestFrameworkValidator.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/9479/diff/
>
>
> Testing
> -------
>
> I've included new unit test + tested on real cluster.
>
>
> Thanks,
>
> Jarek Cecho
>
>

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9479/#review16998
-----------------------------------------------------------

Ship it!
Ship It!

- Cheolsoo Park
On Feb. 15, 2013, 11:41 p.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9479/
> -----------------------------------------------------------
>
> (Updated Feb. 15, 2013, 11:41 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> I've improved the Framework validator as suggested.
>
>
> This addresses bug SQOOP-892.
>     https://issues.apache.org/jira/browse/SQOOP-892
>
>
> Diffs
> -----
>
>   core/src/main/java/org/apache/sqoop/framework/FrameworkValidator.java 6f9a6fca7d95de7aa098c2e094fd98bb744a49b6
>   core/src/main/java/org/apache/sqoop/framework/configuration/ExportJobConfiguration.java d533089c4602fd7740502b6d1607ba0716acf6bb
>   core/src/test/java/org/apache/sqoop/framework/TestFrameworkValidator.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/9479/diff/
>
>
> Testing
> -------
>
> I've included new unit test + tested on real cluster.
>
>
> Thanks,
>
> Jarek Cecho
>
>