You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by rj...@gmail.com on 2013/03/04 09:53:01 UTC
Review Request: Sqoop2: Print out warnings in client shell for objects
created with status ACCEPTABLE
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9729/
-----------------------------------------------------------
Review request for Sqoop.
Description
-------
Print out warnings in client shell for objects created with status ACCEPTABLE
This addresses bug SQOOP-920.
https://issues.apache.org/jira/browse/SQOOP-920
Diffs
-----
client/src/main/java/org/apache/sqoop/client/shell/CreateConnectionFunction.java 2c750d3
client/src/main/java/org/apache/sqoop/client/shell/CreateJobFunction.java 1f01cc7
client/src/main/java/org/apache/sqoop/client/shell/SqoopFunction.java 5632bac
client/src/main/java/org/apache/sqoop/client/shell/UpdateConnectionFunction.java dd0df24
client/src/main/java/org/apache/sqoop/client/shell/UpdateJobFunction.java 4fac977
Diff: https://reviews.apache.org/r/9729/diff/
Testing
-------
Done
Thanks,
vasanthkumar
Re: Review Request: Sqoop2: Print out warnings in client shell for objects
created with status ACCEPTABLE
Posted by Jarek Cecho <ja...@apache.org>.
On March 5, 2013, 4:45 a.m., vasanthkumar wrote:
> > Jarcec
>
> Jarek Cecho wrote:
> Hi Vasanth,
> thank you very much for working on this JIRA. Your contribution is greatly appreciated. I wanted to check with you if you are still intending to work on this patch or whether you would prefer if anyone else would take a look?
>
> Jarcec
>
> vasanthkumar wrote:
> Hi Jarcec,
> I have small clarification. Already you have given comment: " "instanceof" much siimpler as it will introduce common ancestor to both MConnection and Mjob classes". But MConnection, MJob and MSubmission are sharing same parent. Then Forms are managed in MConnection and MJob, not its ancestor. Please suggest for avoiding the instanceof.
>
> Vasanth kumar
Yeah you're right, I was referring to my planned changes in SQOOP-919 where I've introduced new class MFormList. I think that that particular comment no longer applies.
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9729/#review17376
-----------------------------------------------------------
On March 4, 2013, 8:53 a.m., vasanthkumar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9729/
> -----------------------------------------------------------
>
> (Updated March 4, 2013, 8:53 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> Print out warnings in client shell for objects created with status ACCEPTABLE
>
>
> This addresses bug SQOOP-920.
> https://issues.apache.org/jira/browse/SQOOP-920
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/sqoop/client/shell/CreateConnectionFunction.java 2c750d3
> client/src/main/java/org/apache/sqoop/client/shell/CreateJobFunction.java 1f01cc7
> client/src/main/java/org/apache/sqoop/client/shell/SqoopFunction.java 5632bac
> client/src/main/java/org/apache/sqoop/client/shell/UpdateConnectionFunction.java dd0df24
> client/src/main/java/org/apache/sqoop/client/shell/UpdateJobFunction.java 4fac977
>
> Diff: https://reviews.apache.org/r/9729/diff/
>
>
> Testing
> -------
>
> Done
>
>
> Thanks,
>
> vasanthkumar
>
>
Re: Review Request: Sqoop2: Print out warnings in client shell for objects
created with status ACCEPTABLE
Posted by Jarek Cecho <ja...@apache.org>.
On March 5, 2013, 4:45 a.m., vasanthkumar wrote:
> > Jarcec
Hi Vasanth,
thank you very much for working on this JIRA. Your contribution is greatly appreciated. I wanted to check with you if you are still intending to work on this patch or whether you would prefer if anyone else would take a look?
Jarcec
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9729/#review17376
-----------------------------------------------------------
On March 4, 2013, 8:53 a.m., vasanthkumar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9729/
> -----------------------------------------------------------
>
> (Updated March 4, 2013, 8:53 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> Print out warnings in client shell for objects created with status ACCEPTABLE
>
>
> This addresses bug SQOOP-920.
> https://issues.apache.org/jira/browse/SQOOP-920
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/sqoop/client/shell/CreateConnectionFunction.java 2c750d3
> client/src/main/java/org/apache/sqoop/client/shell/CreateJobFunction.java 1f01cc7
> client/src/main/java/org/apache/sqoop/client/shell/SqoopFunction.java 5632bac
> client/src/main/java/org/apache/sqoop/client/shell/UpdateConnectionFunction.java dd0df24
> client/src/main/java/org/apache/sqoop/client/shell/UpdateJobFunction.java 4fac977
>
> Diff: https://reviews.apache.org/r/9729/diff/
>
>
> Testing
> -------
>
> Done
>
>
> Thanks,
>
> vasanthkumar
>
>
Re: Review Request: Sqoop2: Print out warnings in client shell for objects
created with status ACCEPTABLE
Posted by rj...@gmail.com.
On March 5, 2013, 4:45 a.m., vasanthkumar wrote:
> > Jarcec
>
> Jarek Cecho wrote:
> Hi Vasanth,
> thank you very much for working on this JIRA. Your contribution is greatly appreciated. I wanted to check with you if you are still intending to work on this patch or whether you would prefer if anyone else would take a look?
>
> Jarcec
Hi Jarcec,
I have small clarification. Already you have given comment: " "instanceof" much siimpler as it will introduce common ancestor to both MConnection and Mjob classes". But MConnection, MJob and MSubmission are sharing same parent. Then Forms are managed in MConnection and MJob, not its ancestor. Please suggest for avoiding the instanceof.
Vasanth kumar
- vasanthkumar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9729/#review17376
-----------------------------------------------------------
On March 4, 2013, 8:53 a.m., vasanthkumar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9729/
> -----------------------------------------------------------
>
> (Updated March 4, 2013, 8:53 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> Print out warnings in client shell for objects created with status ACCEPTABLE
>
>
> This addresses bug SQOOP-920.
> https://issues.apache.org/jira/browse/SQOOP-920
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/sqoop/client/shell/CreateConnectionFunction.java 2c750d3
> client/src/main/java/org/apache/sqoop/client/shell/CreateJobFunction.java 1f01cc7
> client/src/main/java/org/apache/sqoop/client/shell/SqoopFunction.java 5632bac
> client/src/main/java/org/apache/sqoop/client/shell/UpdateConnectionFunction.java dd0df24
> client/src/main/java/org/apache/sqoop/client/shell/UpdateJobFunction.java 4fac977
>
> Diff: https://reviews.apache.org/r/9729/diff/
>
>
> Testing
> -------
>
> Done
>
>
> Thanks,
>
> vasanthkumar
>
>
Re: Review Request: Sqoop2: Print out warnings in client shell for objects
created with status ACCEPTABLE
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9729/#review17376
-----------------------------------------------------------
Hi Vasant,
thank you very much for working on this!
client/src/main/java/org/apache/sqoop/client/shell/SqoopFunction.java
<https://reviews.apache.org/r/9729/#comment36891>
Would you mind putting this logic to some util class rather than to SqoopFunction? There is couple of output functions in FormDisplayer class already, so it might be a good candidate.
client/src/main/java/org/apache/sqoop/client/shell/SqoopFunction.java
<https://reviews.apache.org/r/9729/#comment36892>
I'm currently working on a patch that will make this "instanceof" much siimpler as it will introduce common ancestor to both MConnection and Mjob classes.
client/src/main/java/org/apache/sqoop/client/shell/SqoopFunction.java
<https://reviews.apache.org/r/9729/#comment36893>
Can we make it more "user friendly"? Something like saying "there were some issues, here is their list, but hey, don't worry, we saved the object anyway".
Jarcec
- Jarek Cecho
On March 4, 2013, 8:53 a.m., vasanthkumar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9729/
> -----------------------------------------------------------
>
> (Updated March 4, 2013, 8:53 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> Print out warnings in client shell for objects created with status ACCEPTABLE
>
>
> This addresses bug SQOOP-920.
> https://issues.apache.org/jira/browse/SQOOP-920
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/sqoop/client/shell/CreateConnectionFunction.java 2c750d3
> client/src/main/java/org/apache/sqoop/client/shell/CreateJobFunction.java 1f01cc7
> client/src/main/java/org/apache/sqoop/client/shell/SqoopFunction.java 5632bac
> client/src/main/java/org/apache/sqoop/client/shell/UpdateConnectionFunction.java dd0df24
> client/src/main/java/org/apache/sqoop/client/shell/UpdateJobFunction.java 4fac977
>
> Diff: https://reviews.apache.org/r/9729/diff/
>
>
> Testing
> -------
>
> Done
>
>
> Thanks,
>
> vasanthkumar
>
>
Re: Review Request: Sqoop2: Print out warnings in client shell for objects
created with status ACCEPTABLE
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9729/#review18408
-----------------------------------------------------------
Ship it!
Ship It!
- Jarek Cecho
On March 26, 2013, 8:32 a.m., vasanthkumar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9729/
> -----------------------------------------------------------
>
> (Updated March 26, 2013, 8:32 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> Print out warnings in client shell for objects created with status ACCEPTABLE
>
>
> This addresses bug SQOOP-920.
> https://issues.apache.org/jira/browse/SQOOP-920
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/sqoop/client/core/Constants.java b93577c
> client/src/main/java/org/apache/sqoop/client/shell/CreateConnectionFunction.java 5bf9aaa
> client/src/main/java/org/apache/sqoop/client/shell/CreateJobFunction.java f5155d5
> client/src/main/java/org/apache/sqoop/client/shell/UpdateConnectionFunction.java 95ada63
> client/src/main/java/org/apache/sqoop/client/shell/UpdateJobFunction.java 3f0ac7d
> client/src/main/java/org/apache/sqoop/client/utils/FormDisplayer.java 7cce713
> client/src/main/resources/client-resource.properties 7e68c4f
>
> Diff: https://reviews.apache.org/r/9729/diff/
>
>
> Testing
> -------
>
> Done
>
>
> Thanks,
>
> vasanthkumar
>
>
Re: Review Request: Sqoop2: Print out warnings in client shell for objects
created with status ACCEPTABLE
Posted by rj...@gmail.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9729/
-----------------------------------------------------------
(Updated March 26, 2013, 8:32 a.m.)
Review request for Sqoop.
Changes
-------
Jarcec comments applied.
Description
-------
Print out warnings in client shell for objects created with status ACCEPTABLE
This addresses bug SQOOP-920.
https://issues.apache.org/jira/browse/SQOOP-920
Diffs (updated)
-----
client/src/main/java/org/apache/sqoop/client/core/Constants.java b93577c
client/src/main/java/org/apache/sqoop/client/shell/CreateConnectionFunction.java 5bf9aaa
client/src/main/java/org/apache/sqoop/client/shell/CreateJobFunction.java f5155d5
client/src/main/java/org/apache/sqoop/client/shell/UpdateConnectionFunction.java 95ada63
client/src/main/java/org/apache/sqoop/client/shell/UpdateJobFunction.java 3f0ac7d
client/src/main/java/org/apache/sqoop/client/utils/FormDisplayer.java 7cce713
client/src/main/resources/client-resource.properties 7e68c4f
Diff: https://reviews.apache.org/r/9729/diff/
Testing
-------
Done
Thanks,
vasanthkumar