Re: Review Request 17817: OOZIE-1689 HA support for OOZIE-7(Ability to view the log information corresponding to particular coordinator action)

2014-05-20 Thread Mona Chitnis 

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

Ship it!


Ship It!

- Mona Chitnis


On Feb. 13, 2014, 12:32 a.m., Purshotam Shah wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17817/
> ---
> 
> (Updated Feb. 13, 2014, 12:32 a.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-1689
> https://issues.apache.org/jira/browse/OOZIE-1689
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> HA support for OOZIE-7(Ability to view the log information corresponding to 
> particular coordinator action)
> 
> 
> Diffs
> -
> 
>   core/src/main/java/org/apache/oozie/service/ZKXLogStreamingService.java 
> 8dc8b4b 
>   core/src/main/java/org/apache/oozie/util/AuthUrlClient.java 79f8883 
>   core/src/test/java/org/apache/oozie/service/TestZKXLogStreamingService.java 
> 29bca41 
> 
> Diff: https://reviews.apache.org/r/17817/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Purshotam Shah
> 
>

Re: Review Request 17817: OOZIE-1689 HA support for OOZIE-7(Ability to view the log information corresponding to particular coordinator action)

2014-03-26 Thread Mona Chitnis 

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


minor comments. +1 after addressing them


core/src/main/java/org/apache/oozie/service/ZKXLogStreamingService.java


Why is params map value data structure string array if we do not support 
multi values right now? can be added later if such design is added.



core/src/main/java/org/apache/oozie/util/AuthUrlClient.java


change to more meaningful name such as getQueryParamString()


- Mona Chitnis


On Feb. 13, 2014, 12:32 a.m., Purshotam Shah wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17817/
> ---
> 
> (Updated Feb. 13, 2014, 12:32 a.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-1689
> https://issues.apache.org/jira/browse/OOZIE-1689
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> HA support for OOZIE-7(Ability to view the log information corresponding to 
> particular coordinator action)
> 
> 
> Diffs
> -
> 
>   core/src/main/java/org/apache/oozie/service/ZKXLogStreamingService.java 
> 8dc8b4b 
>   core/src/main/java/org/apache/oozie/util/AuthUrlClient.java 79f8883 
>   core/src/test/java/org/apache/oozie/service/TestZKXLogStreamingService.java 
> 29bca41 
> 
> Diff: https://reviews.apache.org/r/17817/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Purshotam Shah
> 
>

Re: Review Request 17817: OOZIE-1689 HA support for OOZIE-7(Ability to view the log information corresponding to particular coordinator action)

2014-02-12 Thread Purshotam Shah 

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

(Updated Feb. 13, 2014, 12:32 a.m.)


Review request for oozie.


Changes
---

removing redundant null check.


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


Repository: oozie-git


Description
---

HA support for OOZIE-7(Ability to view the log information corresponding to 
particular coordinator action)


Diffs (updated)
-

  core/src/main/java/org/apache/oozie/service/ZKXLogStreamingService.java 
8dc8b4b 
  core/src/main/java/org/apache/oozie/util/AuthUrlClient.java 79f8883 
  core/src/test/java/org/apache/oozie/service/TestZKXLogStreamingService.java 
29bca41 

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


Testing
---


Thanks,

Purshotam Shah

Re: Review Request 17817: OOZIE-1689 HA support for OOZIE-7(Ability to view the log information corresponding to particular coordinator action)

2014-02-12 Thread Purshotam Shah 


> On Feb. 13, 2014, 12:06 a.m., Mona Chitnis wrote:
> > core/src/main/java/org/apache/oozie/util/AuthUrlClient.java, line 147
> > 
> >
> > As I see it, the total query param is going to look like 
> > ALL_SERVERS_PARAM=false&key=&key=.. 
> > 
> > Can you elaborate why value is null and what NPE are you trying to 
> > avoid?

Null check is redundant here, removed it.


- Purshotam


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


On Feb. 13, 2014, 12:32 a.m., Purshotam Shah wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17817/
> ---
> 
> (Updated Feb. 13, 2014, 12:32 a.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-1689
> https://issues.apache.org/jira/browse/OOZIE-1689
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> HA support for OOZIE-7(Ability to view the log information corresponding to 
> particular coordinator action)
> 
> 
> Diffs
> -
> 
>   core/src/main/java/org/apache/oozie/service/ZKXLogStreamingService.java 
> 8dc8b4b 
>   core/src/main/java/org/apache/oozie/util/AuthUrlClient.java 79f8883 
>   core/src/test/java/org/apache/oozie/service/TestZKXLogStreamingService.java 
> 29bca41 
> 
> Diff: https://reviews.apache.org/r/17817/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Purshotam Shah
> 
>

Re: Review Request 17817: OOZIE-1689 HA support for OOZIE-7(Ability to view the log information corresponding to particular coordinator action)

2014-02-12 Thread Mona Chitnis 

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



core/src/main/java/org/apache/oozie/util/AuthUrlClient.java


okay



core/src/main/java/org/apache/oozie/util/AuthUrlClient.java


As I see it, the total query param is going to look like 
ALL_SERVERS_PARAM=false&key=&key=.. 

Can you elaborate why value is null and what NPE are you trying to avoid?


- Mona Chitnis


On Feb. 6, 2014, 9:10 p.m., Purshotam Shah wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17817/
> ---
> 
> (Updated Feb. 6, 2014, 9:10 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-1689
> https://issues.apache.org/jira/browse/OOZIE-1689
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> HA support for OOZIE-7(Ability to view the log information corresponding to 
> particular coordinator action)
> 
> 
> Diffs
> -
> 
>   core/src/main/java/org/apache/oozie/service/ZKXLogStreamingService.java 
> 8dc8b4b 
>   core/src/main/java/org/apache/oozie/util/AuthUrlClient.java 79f8883 
>   core/src/test/java/org/apache/oozie/service/TestZKXLogStreamingService.java 
> 29bca41 
> 
> Diff: https://reviews.apache.org/r/17817/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Purshotam Shah
> 
>

Re: Review Request 17817: OOZIE-1689 HA support for OOZIE-7(Ability to view the log information corresponding to particular coordinator action)

2014-02-07 Thread Purshotam Shah 

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



core/src/main/java/org/apache/oozie/util/AuthUrlClient.java


This class will be used for communicating with other server.

Other call may also need to convert paramToString. so, it better to put it 
here so that other can use it and share same logic.



core/src/main/java/org/apache/oozie/util/AuthUrlClient.java


If value it null, then it will "key=&".

It good to propagate same key/value even if value is empty to avoid NPE at 
server side.


- Purshotam Shah


On Feb. 6, 2014, 9:10 p.m., Purshotam Shah wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17817/
> ---
> 
> (Updated Feb. 6, 2014, 9:10 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-1689
> https://issues.apache.org/jira/browse/OOZIE-1689
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> HA support for OOZIE-7(Ability to view the log information corresponding to 
> particular coordinator action)
> 
> 
> Diffs
> -
> 
>   core/src/main/java/org/apache/oozie/service/ZKXLogStreamingService.java 
> 8dc8b4b 
>   core/src/main/java/org/apache/oozie/util/AuthUrlClient.java 79f8883 
>   core/src/test/java/org/apache/oozie/service/TestZKXLogStreamingService.java 
> 29bca41 
> 
> Diff: https://reviews.apache.org/r/17817/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Purshotam Shah
> 
>

Re: Review Request 17817: OOZIE-1689 HA support for OOZIE-7(Ability to view the log information corresponding to particular coordinator action)

2014-02-07 Thread Mona Chitnis 

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



core/src/main/java/org/apache/oozie/util/AuthUrlClient.java


this method seems out of place in this Auth.. class. You can place it in 
the LogStreaming class itself



core/src/main/java/org/apache/oozie/util/AuthUrlClient.java


should not have "key=;" inserted when value is null. null check for value 
before appending key itself


- Mona Chitnis


On Feb. 6, 2014, 9:10 p.m., Purshotam Shah wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17817/
> ---
> 
> (Updated Feb. 6, 2014, 9:10 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-1689
> https://issues.apache.org/jira/browse/OOZIE-1689
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> HA support for OOZIE-7(Ability to view the log information corresponding to 
> particular coordinator action)
> 
> 
> Diffs
> -
> 
>   core/src/main/java/org/apache/oozie/service/ZKXLogStreamingService.java 
> 8dc8b4b 
>   core/src/main/java/org/apache/oozie/util/AuthUrlClient.java 79f8883 
>   core/src/test/java/org/apache/oozie/service/TestZKXLogStreamingService.java 
> 29bca41 
> 
> Diff: https://reviews.apache.org/r/17817/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Purshotam Shah
> 
>