> On July 20, 2012, 2:46 a.m., Santhosh Srinivasan wrote:
> > contrib/piggybank/java/src/test/java/org/apache/pig/piggybank/test/storage/avro/TestAvroStorage.java,
> > line 200
> > <https://reviews.apache.org/r/5936/diff/1-2/?file=122600#file122600line200>
> >
> > Can you add an assert here that checks for the expected message from
> > the exception?
The exception thrown by AvroStorage is catched by the Pig backend, and
JobCreationException is re-thrown as follows. I added an assert that compares
the message of the exception against JobCreationException.
Internal error creating job configuration.
org.apache.pig.backend.hadoop.executionengine.mapReduceLayer.JobCreationException:
ERROR 2017: Internal error creating job configuration.
at
org.apache.pig.backend.hadoop.executionengine.mapReduceLayer.JobControlCompiler.getJob(JobControlCompiler.java:756)
at
org.apache.pig.backend.hadoop.executionengine.mapReduceLayer.JobControlCompiler.compile(JobControlCompiler.java:281)
at
org.apache.pig.backend.hadoop.executionengine.mapReduceLayer.MapReduceLauncher.launchPig(MapReduceLauncher.java:179)
at org.apache.pig.PigServer.launchPlan(PigServer.java:1260)
at org.apache.pig.PigServer.executeCompiledLogicalPlan(PigServer.java:1245)
at org.apache.pig.PigServer.execute(PigServer.java:1235)
at org.apache.pig.PigServer.executeBatch(PigServer.java:329)
at
org.apache.pig.piggybank.test.storage.avro.TestAvroStorage.testAvroStorage(TestAvroStorage.java:495)
at
org.apache.pig.piggybank.test.storage.avro.TestAvroStorage.testGlob6(TestAvroStorage.java:197)
Caused by: java.io.IOException: Input path
'file:///home/cheolsoo/workspace/pig-git/contrib/piggybank/java/src/test/java/org/apache/pig/piggybank/test/storage/avro/avro_test_files/test_dir{1,2}/file_that_does_not_exist*.avro'
is not found
at
org.apache.pig.piggybank.storage.avro.AvroStorage.setLocation(AvroStorage.java:142)
at
org.apache.pig.backend.hadoop.executionengine.mapReduceLayer.JobControlCompiler.getJob(JobControlCompiler.java:403)
> On July 20, 2012, 2:46 a.m., Santhosh Srinivasan wrote:
> > contrib/piggybank/java/src/test/java/org/apache/pig/piggybank/test/storage/avro/TestAvroStorageUtils.java,
> > line 161
> > <https://reviews.apache.org/r/5936/diff/2/?file=124635#file124635line161>
> >
> > Can you add an assert to check for the message from the exception?
The message of the exception looks like this. Since this is a bit too long, I
simply confirm that it contains 'Illegal file pattern'.
Illegal file pattern: Unclosed group near index 3
{1,
^
> On July 20, 2012, 2:46 a.m., Santhosh Srinivasan wrote:
> > contrib/piggybank/java/src/test/java/org/apache/pig/piggybank/test/storage/avro/TestAvroStorageUtils.java,
> > line 163
> > <https://reviews.apache.org/r/5936/diff/2/?file=124635#file124635line163>
> >
> > Why is this check required? What are the guarantees for concretePath
> > being null. There is an assumption here that concretePath is/was null. It
> > could be due to the previous test setting concretePath to null.
This was a mistake. I removed it.
> On July 20, 2012, 2:46 a.m., Santhosh Srinivasan wrote:
> > contrib/piggybank/java/src/test/java/org/apache/pig/piggybank/test/storage/avro/TestAvroStorage.java,
> > line 173
> > <https://reviews.apache.org/r/5936/diff/1-2/?file=122600#file122600line173>
> >
> > escaped
Fixed
> On July 20, 2012, 2:46 a.m., Santhosh Srinivasan wrote:
> > contrib/piggybank/java/src/test/java/org/apache/pig/piggybank/test/storage/avro/TestAvroStorage.java,
> > line 158
> > <https://reviews.apache.org/r/5936/diff/1-2/?file=122600#file122600line158>
> >
> > escaped
Fixed
> On July 20, 2012, 2:46 a.m., Santhosh Srinivasan wrote:
> > contrib/piggybank/java/src/test/java/org/apache/pig/piggybank/test/storage/avro/TestAvroStorage.java,
> > line 143
> > <https://reviews.apache.org/r/5936/diff/1-2/?file=122600#file122600line143>
> >
> > escaped
Fixed
> On July 20, 2012, 2:46 a.m., Santhosh Srinivasan wrote:
> > contrib/piggybank/java/src/test/java/org/apache/pig/piggybank/test/storage/avro/TestAvroStorage.java,
> > line 128
> > <https://reviews.apache.org/r/5936/diff/1-2/?file=122600#file122600line128>
> >
> > escaped
Fixed
> On July 20, 2012, 2:46 a.m., Santhosh Srinivasan wrote:
> > contrib/piggybank/java/src/test/java/org/apache/pig/piggybank/test/storage/avro/TestAvroStorage.java,
> > line 71
> > <https://reviews.apache.org/r/5936/diff/1-2/?file=122600#file122600line71>
> >
> > Looks like this variable is not used.
Removed
- Cheolsoo
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5936/#review9293
-----------------------------------------------------------
On July 19, 2012, 1:23 a.m., Cheolsoo Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5936/
> -----------------------------------------------------------
>
> (Updated July 19, 2012, 1:23 a.m.)
>
>
> Review request for pig.
>
>
> Description
> -------
>
> Add glob support to AvroStorage:
>
> https://issues.apache.org/jira/browse/PIG-2492
>
>
> This addresses bug PIG-2492.
> https://issues.apache.org/jira/browse/PIG-2492
>
>
> Diffs
> -----
>
>
> contrib/piggybank/java/src/main/java/org/apache/pig/piggybank/storage/avro/AvroStorage.java
> 0f8ef27
>
> contrib/piggybank/java/src/main/java/org/apache/pig/piggybank/storage/avro/AvroStorageUtils.java
> c7de726
>
> contrib/piggybank/java/src/test/java/org/apache/pig/piggybank/test/storage/avro/TestAvroStorage.java
> 48b093b
>
> contrib/piggybank/java/src/test/java/org/apache/pig/piggybank/test/storage/avro/TestAvroStorageUtils.java
> e5d0c38
>
> Diff: https://reviews.apache.org/r/5936/diff/
>
>
> Testing
> -------
>
> 1. Added new unit tests as follows:
>
> - testDir verifies that AvroStorage recursively loads files in a directory
> and its sub-directories.
> - testGlob1 to 3 verify that glob patterns are expanded properly.
>
> To run the tests, please do the following:
>
> wget
> https://issues.apache.org/jira/secure/attachment/12536534/avro_test_files.tar.gz
>
> tar -xf avro_test_files.tar.gz
> ant clean compile-test piggybank -Dhadoopversion=20
> cd contrib/piggybank/java
> ant test -Dtestcase=TestAvroStorage
>
> 2. Both TestAvroStorage and TestAvroStorageUtils pass.
>
>
> Thanks,
>
> Cheolsoo Park
>
>
