You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Linden Hillenbrand <li...@cloudera.com> on 2013/02/18 17:32:26 UTC
Review Request: Cleaned up error codes in MapreduceExecutionError
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9495/
-----------------------------------------------------------
Review request for Sqoop, Jarek Cecho and Kathleen Ting.
Description
-------
Checked each error in MapreduceExecutionError.java and kept ones referenced in the code base and cleaned up the ones that were not being used.
The ones I was able to remove are the following:
/** Error occurs during job execution. */
MAPRED_EXEC_0008("Error occurs during job execution"),
/** The system was unable to load the specified class. */
MAPRED_EXEC_0009("Unable to load the specified class"),
/** The parameter already exists in the context */
MAPRED_EXEC_0011("The parameter already exists in the context"),
/** Cannot read from the data reader */
MAPRED_EXEC_0014("Cannot read to the data reader"),
/** Unable to write data due to interrupt */
MAPRED_EXEC_0015("Unable to write data due to interrupt"),
/** Unable to read data due to interrupt */
MAPRED_EXEC_0016("Unable to read data due to interrupt"),
/** The required option has not been set yet */
MAPRED_EXEC_0020("The required option has not been set yet"),
This addresses bug Sqoop-743.
https://issues.apache.org/jira/browse/Sqoop-743
Diffs
-----
execution/mapreduce/src/main/java/org/apache/sqoop/job/MapreduceExecutionError.java 1dc12d1
Diff: https://reviews.apache.org/r/9495/diff/
Testing
-------
Ran all tests and passed successfully.
Thanks,
Linden Hillenbrand
Re: Review Request: Cleaned up error codes in MapreduceExecutionError
Posted by Linden Hillenbrand <li...@cloudera.com>.
On Feb. 18, 2013, 8:31 p.m., Linden Hillenbrand wrote:
> > Jarcec
Hey Jarcec,
I have a quick question (just want to make sure I go about the renumeration correctly), therefore when I renumerate I need to change the error code references in the codebase so they call the proper exceptions. When I grep for example, '0009' which I am about to change to '0001', I get the following:
[linden@localhost sqoop2]$ grep -r -i MAPRED_EXEC_0009 .
./dist/target/sqoop-2.0.0-SNAPSHOT/execution/mapreduce/src/main/java/org/apache/sqoop/job/MapreduceExecutionError.java: MAPRED_EXEC_0009("Unable to load the specified class"),
./dist/target/sqoop-2.0.0-SNAPSHOT/execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopSplit.java: throw new SqoopException(MapreduceExecutionError.MAPRED_EXEC_0009, className);
./dist/target/sqoop-2.0.0-SNAPSHOT/execution/mapreduce/src/main/java/org/apache/sqoop/job/etl/HdfsSequenceImportLoader.java: throw new SqoopException(MapreduceExecutionError.MAPRED_EXEC_0009, codecname);
./dist/target/sqoop-2.0.0-SNAPSHOT/execution/mapreduce/src/main/java/org/apache/sqoop/job/etl/HdfsTextImportLoader.java: throw new SqoopException(MapreduceExecutionError.MAPRED_EXEC_0009, codecname);
Binary file ./execution/mapreduce/target/classes/org/apache/sqoop/job/MapreduceExecutionError.class matches
Binary file ./execution/mapreduce/target/classes/org/apache/sqoop/job/mr/SqoopSplit.class matches
Binary file ./execution/mapreduce/target/classes/org/apache/sqoop/job/etl/HdfsSequenceImportLoader.class matches
Binary file ./execution/mapreduce/target/classes/org/apache/sqoop/job/etl/HdfsTextImportLoader.class matches
./execution/mapreduce/src/main/java/org/apache/sqoop/job/MapreduceExecutionError.java: MAPRED_EXEC_0009("Unable to load the specified class"),
./execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopSplit.java: throw new SqoopException(MapreduceExecutionError.MAPRED_EXEC_0009, className);
./execution/mapreduce/src/main/java/org/apache/sqoop/job/etl/HdfsSequenceImportLoader.java: throw new SqoopException(MapreduceExecutionError.MAPRED_EXEC_0009, codecname);
./execution/mapreduce/src/main/java/org/apache/sqoop/job/etl/HdfsTextImportLoader.java: throw new SqoopException(MapreduceExecutionError.MAPRED_EXEC_0009, codecname);
What I would like to understand is a few things (happy to jump on a call Monday as well if that is easier to answer):
./dist/target/sqoop-2.0.0-SNAPSHOT/execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopSplit.java
vs
./execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopSplit.java
- How are the above to different?
- Is the /dist/target/*-SNAPSHOT just a compiled version of the code that I have on my machine?
- When I make the enumeration change do I need to make it to both files or just one?
- I am guessing the ./execution/mapreduce,...is the actual branch that I want to make the change on.
I appreciate the guidance, I just want to fully understand the change, and if I need to make it in two places then why.
Thank you sir.
- Linden
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9495/#review16714
-----------------------------------------------------------
On Feb. 18, 2013, 4:32 p.m., Linden Hillenbrand wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9495/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2013, 4:32 p.m.)
>
>
> Review request for Sqoop, Jarek Cecho and Kathleen Ting.
>
>
> Description
> -------
>
> Checked each error in MapreduceExecutionError.java and kept ones referenced in the code base and cleaned up the ones that were not being used.
>
> The ones I was able to remove are the following:
>
> /** Error occurs during job execution. */
> MAPRED_EXEC_0008("Error occurs during job execution"),
>
> /** The system was unable to load the specified class. */
> MAPRED_EXEC_0009("Unable to load the specified class"),
>
> /** The parameter already exists in the context */
> MAPRED_EXEC_0011("The parameter already exists in the context"),
>
> /** Cannot read from the data reader */
> MAPRED_EXEC_0014("Cannot read to the data reader"),
>
> /** Unable to write data due to interrupt */
> MAPRED_EXEC_0015("Unable to write data due to interrupt"),
>
> /** Unable to read data due to interrupt */
> MAPRED_EXEC_0016("Unable to read data due to interrupt"),
>
> /** The required option has not been set yet */
> MAPRED_EXEC_0020("The required option has not been set yet"),
>
>
> This addresses bug Sqoop-743.
> https://issues.apache.org/jira/browse/Sqoop-743
>
>
> Diffs
> -----
>
> execution/mapreduce/src/main/java/org/apache/sqoop/job/MapreduceExecutionError.java 1dc12d1
>
> Diff: https://reviews.apache.org/r/9495/diff/
>
>
> Testing
> -------
>
> Ran all tests and passed successfully.
>
>
> Thanks,
>
> Linden Hillenbrand
>
>
Re: Review Request: Cleaned up error codes in MapreduceExecutionError
Posted by Kathleen Ting <ka...@apache.org>.
On Feb. 18, 2013, 8:31 p.m., Linden Hillenbrand wrote:
> > Jarcec
>
> Linden Hillenbrand wrote:
> Hey Jarcec,
>
> I have a quick question (just want to make sure I go about the renumeration correctly), therefore when I renumerate I need to change the error code references in the codebase so they call the proper exceptions. When I grep for example, '0009' which I am about to change to '0001', I get the following:
>
> [linden@localhost sqoop2]$ grep -r -i MAPRED_EXEC_0009 .
> ./dist/target/sqoop-2.0.0-SNAPSHOT/execution/mapreduce/src/main/java/org/apache/sqoop/job/MapreduceExecutionError.java: MAPRED_EXEC_0009("Unable to load the specified class"),
> ./dist/target/sqoop-2.0.0-SNAPSHOT/execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopSplit.java: throw new SqoopException(MapreduceExecutionError.MAPRED_EXEC_0009, className);
> ./dist/target/sqoop-2.0.0-SNAPSHOT/execution/mapreduce/src/main/java/org/apache/sqoop/job/etl/HdfsSequenceImportLoader.java: throw new SqoopException(MapreduceExecutionError.MAPRED_EXEC_0009, codecname);
> ./dist/target/sqoop-2.0.0-SNAPSHOT/execution/mapreduce/src/main/java/org/apache/sqoop/job/etl/HdfsTextImportLoader.java: throw new SqoopException(MapreduceExecutionError.MAPRED_EXEC_0009, codecname);
> Binary file ./execution/mapreduce/target/classes/org/apache/sqoop/job/MapreduceExecutionError.class matches
> Binary file ./execution/mapreduce/target/classes/org/apache/sqoop/job/mr/SqoopSplit.class matches
> Binary file ./execution/mapreduce/target/classes/org/apache/sqoop/job/etl/HdfsSequenceImportLoader.class matches
> Binary file ./execution/mapreduce/target/classes/org/apache/sqoop/job/etl/HdfsTextImportLoader.class matches
> ./execution/mapreduce/src/main/java/org/apache/sqoop/job/MapreduceExecutionError.java: MAPRED_EXEC_0009("Unable to load the specified class"),
> ./execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopSplit.java: throw new SqoopException(MapreduceExecutionError.MAPRED_EXEC_0009, className);
> ./execution/mapreduce/src/main/java/org/apache/sqoop/job/etl/HdfsSequenceImportLoader.java: throw new SqoopException(MapreduceExecutionError.MAPRED_EXEC_0009, codecname);
> ./execution/mapreduce/src/main/java/org/apache/sqoop/job/etl/HdfsTextImportLoader.java: throw new SqoopException(MapreduceExecutionError.MAPRED_EXEC_0009, codecname);
>
> What I would like to understand is a few things (happy to jump on a call Monday as well if that is easier to answer):
>
> ./dist/target/sqoop-2.0.0-SNAPSHOT/execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopSplit.java
> vs
> ./execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopSplit.java
>
> - How are the above to different?
> - Is the /dist/target/*-SNAPSHOT just a compiled version of the code that I have on my machine?
> - When I make the enumeration change do I need to make it to both files or just one?
> - I am guessing the ./execution/mapreduce,...is the actual branch that I want to make the change on.
>
> I appreciate the guidance, I just want to fully understand the change, and if I need to make it in two places then why.
>
> Thank you sir.
Linden, correct, the /dist/target/*-SNAPSHOT is just a compiled version of the code. If you do a 'mvn clean' you'll only see one version of SqoopSplit.java.
- Kathleen
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9495/#review16714
-----------------------------------------------------------
On Feb. 18, 2013, 4:32 p.m., Linden Hillenbrand wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9495/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2013, 4:32 p.m.)
>
>
> Review request for Sqoop, Jarek Cecho and Kathleen Ting.
>
>
> Description
> -------
>
> Checked each error in MapreduceExecutionError.java and kept ones referenced in the code base and cleaned up the ones that were not being used.
>
> The ones I was able to remove are the following:
>
> /** Error occurs during job execution. */
> MAPRED_EXEC_0008("Error occurs during job execution"),
>
> /** The system was unable to load the specified class. */
> MAPRED_EXEC_0009("Unable to load the specified class"),
>
> /** The parameter already exists in the context */
> MAPRED_EXEC_0011("The parameter already exists in the context"),
>
> /** Cannot read from the data reader */
> MAPRED_EXEC_0014("Cannot read to the data reader"),
>
> /** Unable to write data due to interrupt */
> MAPRED_EXEC_0015("Unable to write data due to interrupt"),
>
> /** Unable to read data due to interrupt */
> MAPRED_EXEC_0016("Unable to read data due to interrupt"),
>
> /** The required option has not been set yet */
> MAPRED_EXEC_0020("The required option has not been set yet"),
>
>
> This addresses bug Sqoop-743.
> https://issues.apache.org/jira/browse/Sqoop-743
>
>
> Diffs
> -----
>
> execution/mapreduce/src/main/java/org/apache/sqoop/job/MapreduceExecutionError.java 1dc12d1
>
> Diff: https://reviews.apache.org/r/9495/diff/
>
>
> Testing
> -------
>
> Ran all tests and passed successfully.
>
>
> Thanks,
>
> Linden Hillenbrand
>
>
Re: Review Request: Cleaned up error codes in MapreduceExecutionError
Posted by Linden Hillenbrand <li...@cloudera.com>.
> On Feb. 18, 2013, 8:31 p.m., Jarek Cecho wrote:
> > execution/mapreduce/src/main/java/org/apache/sqoop/job/MapreduceExecutionError.java, line 27
> > <https://reviews.apache.org/r/9495/diff/1/?file=259570#file259570line27>
> >
> > Would you mind keeping this around? I know that it's unused, but we're trying to reserve the error code 0000 to unknown issues across entire code base.
Agreed, I will clean the gaps and also it makes sense to keep '0000' around, I'll re-grep the codebase to see where that is used as well just as an FYI for my sake.
- Linden
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9495/#review16714
-----------------------------------------------------------
On Feb. 18, 2013, 4:32 p.m., Linden Hillenbrand wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9495/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2013, 4:32 p.m.)
>
>
> Review request for Sqoop, Jarek Cecho and Kathleen Ting.
>
>
> Description
> -------
>
> Checked each error in MapreduceExecutionError.java and kept ones referenced in the code base and cleaned up the ones that were not being used.
>
> The ones I was able to remove are the following:
>
> /** Error occurs during job execution. */
> MAPRED_EXEC_0008("Error occurs during job execution"),
>
> /** The system was unable to load the specified class. */
> MAPRED_EXEC_0009("Unable to load the specified class"),
>
> /** The parameter already exists in the context */
> MAPRED_EXEC_0011("The parameter already exists in the context"),
>
> /** Cannot read from the data reader */
> MAPRED_EXEC_0014("Cannot read to the data reader"),
>
> /** Unable to write data due to interrupt */
> MAPRED_EXEC_0015("Unable to write data due to interrupt"),
>
> /** Unable to read data due to interrupt */
> MAPRED_EXEC_0016("Unable to read data due to interrupt"),
>
> /** The required option has not been set yet */
> MAPRED_EXEC_0020("The required option has not been set yet"),
>
>
> This addresses bug Sqoop-743.
> https://issues.apache.org/jira/browse/Sqoop-743
>
>
> Diffs
> -----
>
> execution/mapreduce/src/main/java/org/apache/sqoop/job/MapreduceExecutionError.java 1dc12d1
>
> Diff: https://reviews.apache.org/r/9495/diff/
>
>
> Testing
> -------
>
> Ran all tests and passed successfully.
>
>
> Thanks,
>
> Linden Hillenbrand
>
>
Re: Review Request: Cleaned up error codes in MapreduceExecutionError
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9495/#review16714
-----------------------------------------------------------
Thank you Linden for taking a look into this! I appreciate your time.
The patch looks good to me. Just one idea - since Sqoop2 is still in development phase it might be helpful to renumerate the exceptions (starting with 0000 and increasing by one) to clean the gaps. What do you think?
execution/mapreduce/src/main/java/org/apache/sqoop/job/MapreduceExecutionError.java
<https://reviews.apache.org/r/9495/#comment35431>
Would you mind keeping this around? I know that it's unused, but we're trying to reserve the error code 0000 to unknown issues across entire code base.
Jarcec
- Jarek Cecho
On Feb. 18, 2013, 4:32 p.m., Linden Hillenbrand wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9495/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2013, 4:32 p.m.)
>
>
> Review request for Sqoop, Jarek Cecho and Kathleen Ting.
>
>
> Description
> -------
>
> Checked each error in MapreduceExecutionError.java and kept ones referenced in the code base and cleaned up the ones that were not being used.
>
> The ones I was able to remove are the following:
>
> /** Error occurs during job execution. */
> MAPRED_EXEC_0008("Error occurs during job execution"),
>
> /** The system was unable to load the specified class. */
> MAPRED_EXEC_0009("Unable to load the specified class"),
>
> /** The parameter already exists in the context */
> MAPRED_EXEC_0011("The parameter already exists in the context"),
>
> /** Cannot read from the data reader */
> MAPRED_EXEC_0014("Cannot read to the data reader"),
>
> /** Unable to write data due to interrupt */
> MAPRED_EXEC_0015("Unable to write data due to interrupt"),
>
> /** Unable to read data due to interrupt */
> MAPRED_EXEC_0016("Unable to read data due to interrupt"),
>
> /** The required option has not been set yet */
> MAPRED_EXEC_0020("The required option has not been set yet"),
>
>
> This addresses bug Sqoop-743.
> https://issues.apache.org/jira/browse/Sqoop-743
>
>
> Diffs
> -----
>
> execution/mapreduce/src/main/java/org/apache/sqoop/job/MapreduceExecutionError.java 1dc12d1
>
> Diff: https://reviews.apache.org/r/9495/diff/
>
>
> Testing
> -------
>
> Ran all tests and passed successfully.
>
>
> Thanks,
>
> Linden Hillenbrand
>
>