You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by Purshotam Shah <pu...@yahoo-inc.com> on 2014/02/06 22:10:35 UTC
Review Request 17817: OOZIE-1689 HA support for OOZIE-7(Ability to view the
log information corresponding to particular coordinator action)
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17817/
-----------------------------------------------------------
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)
Posted by Purshotam Shah <pu...@yahoo-inc.com>.
-----------------------------------------------------------
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
<https://reviews.apache.org/r/17817/#comment63902>
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
<https://reviews.apache.org/r/17817/#comment63904>
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)
Posted by Mona Chitnis <mo...@yahoo.in>.
-----------------------------------------------------------
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
<https://reviews.apache.org/r/17817/#comment70950>
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
<https://reviews.apache.org/r/17817/#comment70951>
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)
Posted by Mona Chitnis <mo...@yahoo.in>.
-----------------------------------------------------------
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)
Posted by Purshotam Shah <pu...@yahoo-inc.com>.
-----------------------------------------------------------
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)
Posted by Purshotam Shah <pu...@yahoo-inc.com>.
> On Feb. 13, 2014, 12:06 a.m., Mona Chitnis wrote:
> > core/src/main/java/org/apache/oozie/util/AuthUrlClient.java, line 147
> > <https://reviews.apache.org/r/17817/diff/2/?file=471083#file471083line147>
> >
> > 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)
Posted by Mona Chitnis <mo...@yahoo.in>.
-----------------------------------------------------------
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
<https://reviews.apache.org/r/17817/#comment64385>
okay
core/src/main/java/org/apache/oozie/util/AuthUrlClient.java
<https://reviews.apache.org/r/17817/#comment64386>
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)
Posted by Mona Chitnis <mo...@yahoo.in>.
-----------------------------------------------------------
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
<https://reviews.apache.org/r/17817/#comment63885>
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
<https://reviews.apache.org/r/17817/#comment63886>
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
>
>