Review Board 1.7.22


Cleaned up error codes in MapreduceExecutionError

Review Request #9495 - Created Feb. 18, 2013 and updated

Linden Hillenbrand
Sqoop-743
Reviewers
Sqoop
jarcec, kate
sqoop-sqoop2
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"),
Ran all tests and passed successfully.
Posted (Feb. 18, 2013, 8:31 p.m.)
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?
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.
  1. 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.
Jarcec
  1. 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.
  2. 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.