Re: Review Request 55404: OOZIE-2701: Oozie to support Multiple HCatalog URIs

2017-03-23 Thread Abhishek Bafna 

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

(Updated March 23, 2017, 9:33 a.m.)


Review request for oozie.


Bugs: OOZIE-2701
https://issues.apache.org/jira/browse/OOZIE-2701


Repository: oozie-git


Description
---

Oozie to support Multiple HCatalog URIs


Diffs (updated)
-

  core/src/main/java/org/apache/oozie/coord/HCatELFunctions.java 9475f72 
  core/src/main/java/org/apache/oozie/dependency/HCatURIHandler.java 4cc284a 
  core/src/main/resources/oozie-default.xml 03f428a 
  sharelib/hcatalog/src/main/java/org/apache/oozie/util/HCatURI.java faeff2a 
  sharelib/hcatalog/src/main/java/org/apache/oozie/util/HCatURIParser.java 
PRE-CREATION 
  sharelib/hcatalog/src/test/java/org/apache/oozie/util/TestHCatURI.java 
ce690da 
  sharelib/hcatalog/src/test/java/org/apache/oozie/util/TestHCatURIParser.java 
PRE-CREATION 


Diff: https://reviews.apache.org/r/55404/diff/3/

Changes: https://reviews.apache.org/r/55404/diff/2-3/


Testing
---


Thanks,

Abhishek Bafna

Re: Review Request 55404: OOZIE-2701: Oozie to support Multiple HCatalog URIs

2017-01-12 Thread András Piros 

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


Ship it!




Ship It!

- András Piros


On Jan. 12, 2017, 8:25 a.m., Abhishek Bafna wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55404/
> ---
> 
> (Updated Jan. 12, 2017, 8:25 a.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-2701
> https://issues.apache.org/jira/browse/OOZIE-2701
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> Oozie to support Multiple HCatalog URIs
> 
> 
> Diffs
> -
> 
>   core/src/main/java/org/apache/oozie/coord/HCatELFunctions.java 9475f72 
>   core/src/main/java/org/apache/oozie/dependency/HCatURIHandler.java 67b37ec 
>   core/src/main/resources/oozie-default.xml 943f9bc 
>   sharelib/hcatalog/src/main/java/org/apache/oozie/util/HCatURI.java 8ec3fae 
>   sharelib/hcatalog/src/main/java/org/apache/oozie/util/HCatURIParser.java 
> PRE-CREATION 
>   sharelib/hcatalog/src/test/java/org/apache/oozie/util/TestHCatURI.java 
> 1b1a8fa 
>   
> sharelib/hcatalog/src/test/java/org/apache/oozie/util/TestHCatURIParser.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/55404/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Abhishek Bafna
> 
>

Re: Review Request 55404: OOZIE-2701: Oozie to support Multiple HCatalog URIs

2017-01-12 Thread Abhishek Bafna 

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

(Updated Jan. 12, 2017, 8:25 a.m.)


Review request for oozie.


Bugs: OOZIE-2701
https://issues.apache.org/jira/browse/OOZIE-2701


Repository: oozie-git


Description
---

Oozie to support Multiple HCatalog URIs


Diffs (updated)
-

  core/src/main/java/org/apache/oozie/coord/HCatELFunctions.java 9475f72 
  core/src/main/java/org/apache/oozie/dependency/HCatURIHandler.java 67b37ec 
  core/src/main/resources/oozie-default.xml 943f9bc 
  sharelib/hcatalog/src/main/java/org/apache/oozie/util/HCatURI.java 8ec3fae 
  sharelib/hcatalog/src/main/java/org/apache/oozie/util/HCatURIParser.java 
PRE-CREATION 
  sharelib/hcatalog/src/test/java/org/apache/oozie/util/TestHCatURI.java 
1b1a8fa 
  sharelib/hcatalog/src/test/java/org/apache/oozie/util/TestHCatURIParser.java 
PRE-CREATION 

Diff: https://reviews.apache.org/r/55404/diff/


Testing
---


Thanks,

Abhishek Bafna

Re: Review Request 55404: OOZIE-2701: Oozie to support Multiple HCatalog URIs

2017-01-12 Thread Abhishek Bafna 


> On Jan. 11, 2017, 11:10 a.m., András Piros wrote:
> > sharelib/hcatalog/src/main/java/org/apache/oozie/util/HCatURI.java, lines 
> > 118-131
> > 
> >
> > Should be in `class HCatURIParser`.

Renamed the method iwht more specific name. I think we should keep this into 
the HCatURI class only as URI is getting pasrsed during the initialization.


> On Jan. 11, 2017, 11:10 a.m., András Piros wrote:
> > core/src/main/java/org/apache/oozie/coord/HCatELFunctions.java, lines 
> > 325-336
> > 
> >
> > Extract to separate `class HCatURIParser`. Should not be `static`.

Moved. Keeping it static as it is used as a helper method and does not have 
state to hold for parsing.


> On Jan. 11, 2017, 11:10 a.m., András Piros wrote:
> > sharelib/hcatalog/src/main/java/org/apache/oozie/util/HCatURI.java, lines 
> > 56-62
> > 
> >
> > Should be in `class HCatURIParser`.

Moved. Marked as static method which parse the URI and retrun URI.


> On Jan. 11, 2017, 11:10 a.m., András Piros wrote:
> > sharelib/hcatalog/src/test/java/org/apache/oozie/util/TestHCatURI.java, 
> > lines 55-57
> > 
> >
> > JUnit 4's `@Rule public ExpectedException expected = 
> > ExpectedException.none(); expected.expect(HCatAccessorException.class);` 
> > comes to the rescue.

Removed the try-catch. Added the 'Exception' as part of the test method 
signatore, if any exception occurs then test should fail and should be analyzed 
and fixed.


- Abhishek


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


On Jan. 11, 2017, 5:41 a.m., Abhishek Bafna wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55404/
> ---
> 
> (Updated Jan. 11, 2017, 5:41 a.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-2701
> https://issues.apache.org/jira/browse/OOZIE-2701
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> Oozie to support Multiple HCatalog URIs
> 
> 
> Diffs
> -
> 
>   core/src/main/java/org/apache/oozie/coord/HCatELFunctions.java 9475f72 
>   core/src/main/java/org/apache/oozie/dependency/HCatURIHandler.java 67b37ec 
>   core/src/main/resources/oozie-default.xml 943f9bc 
>   core/src/test/java/org/apache/oozie/coord/TestHCatELFunctions.java e1cf133 
>   sharelib/hcatalog/src/main/java/org/apache/oozie/util/HCatURI.java 8ec3fae 
>   sharelib/hcatalog/src/test/java/org/apache/oozie/util/TestHCatURI.java 
> 1b1a8fa 
> 
> Diff: https://reviews.apache.org/r/55404/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Abhishek Bafna
> 
>

Re: Review Request 55404: OOZIE-2701: Oozie to support Multiple HCatalog URIs

2017-01-11 Thread András Piros 

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




core/src/main/java/org/apache/oozie/coord/HCatELFunctions.java (line 45)


You can inline that as never used outside the next line.



core/src/main/java/org/apache/oozie/coord/HCatELFunctions.java (line 46)


Pls. rename to `HCAT_URI_REGEX_CONFIG`.



core/src/main/java/org/apache/oozie/coord/HCatELFunctions.java (lines 325 - 336)


Extract to separate `class HCatURIParser`. Should not be `static`.



core/src/main/resources/oozie-default.xml (lines 2967 - 2970)


`` missing.



core/src/test/java/org/apache/oozie/coord/TestHCatELFunctions.java (line 560)


`whenMultipleHCatURIsAreSplitPartsAreExtractedCorrectly()` would be a 
better name.



core/src/test/java/org/apache/oozie/coord/TestHCatELFunctions.java (lines 561 - 
565)


Extract method.



core/src/test/java/org/apache/oozie/coord/TestHCatELFunctions.java (lines 569 - 
572)


Use extracted method.



core/src/test/java/org/apache/oozie/coord/TestHCatELFunctions.java (lines 574 - 
577)


Use extracted method.



sharelib/hcatalog/src/main/java/org/apache/oozie/util/HCatURI.java (lines 56 - 
62)


Should be in `class HCatURIParser`.



sharelib/hcatalog/src/main/java/org/apache/oozie/util/HCatURI.java (lines 64 - 
73)


Should be in `class HCatURIParser`.



sharelib/hcatalog/src/main/java/org/apache/oozie/util/HCatURI.java (lines 118 - 
131)


Should be in `class HCatURIParser`.



sharelib/hcatalog/src/test/java/org/apache/oozie/util/TestHCatURI.java (line 49)


`whenMultipleHCatURIsAreParsedASingleURIIsExtracted()` would be a better 
name.



sharelib/hcatalog/src/test/java/org/apache/oozie/util/TestHCatURI.java (lines 
55 - 57)


JUnit 4's `@Rule public ExpectedException expected = 
ExpectedException.none(); expected.expect(HCatAccessorException.class);` comes 
to the rescue.



sharelib/hcatalog/src/test/java/org/apache/oozie/util/TestHCatURI.java (line 56)


`expected.expectMessage(...)`. Also please do use `Logger` within test 
classes.


- András Piros


On Jan. 11, 2017, 5:41 a.m., Abhishek Bafna wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55404/
> ---
> 
> (Updated Jan. 11, 2017, 5:41 a.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-2701
> https://issues.apache.org/jira/browse/OOZIE-2701
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> Oozie to support Multiple HCatalog URIs
> 
> 
> Diffs
> -
> 
>   core/src/main/java/org/apache/oozie/coord/HCatELFunctions.java 9475f72 
>   core/src/main/java/org/apache/oozie/dependency/HCatURIHandler.java 67b37ec 
>   core/src/main/resources/oozie-default.xml 943f9bc 
>   core/src/test/java/org/apache/oozie/coord/TestHCatELFunctions.java e1cf133 
>   sharelib/hcatalog/src/main/java/org/apache/oozie/util/HCatURI.java 8ec3fae 
>   sharelib/hcatalog/src/test/java/org/apache/oozie/util/TestHCatURI.java 
> 1b1a8fa 
> 
> Diff: https://reviews.apache.org/r/55404/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Abhishek Bafna
> 
>

Review Request 55404: OOZIE-2701: Oozie to support Multiple HCatalog URIs

2017-01-10 Thread Abhishek Bafna 

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

Review request for oozie.


Bugs: OOZIE-2701
https://issues.apache.org/jira/browse/OOZIE-2701


Repository: oozie-git


Description
---

Oozie to support Multiple HCatalog URIs


Diffs
-

  core/src/main/java/org/apache/oozie/coord/HCatELFunctions.java 9475f72 
  core/src/main/java/org/apache/oozie/dependency/HCatURIHandler.java 67b37ec 
  core/src/main/resources/oozie-default.xml 943f9bc 
  core/src/test/java/org/apache/oozie/coord/TestHCatELFunctions.java e1cf133 
  sharelib/hcatalog/src/main/java/org/apache/oozie/util/HCatURI.java 8ec3fae 
  sharelib/hcatalog/src/test/java/org/apache/oozie/util/TestHCatURI.java 
1b1a8fa 

Diff: https://reviews.apache.org/r/55404/diff/


Testing
---


Thanks,

Abhishek Bafna