> On March 9, 2017, 1:36 p.m., András Piros wrote:
> > core/src/main/java/org/apache/oozie/BaseEngine.java
> > Line 192 (original), 197-203 (patched)
> > <https://reviews.apache.org/r/57107/diff/1/?file=1659376#file1659376line202>
> >
> > Code duplication: can you just please use template method pattern?
>
> Purshotam Shah wrote:
> It's not duplicate code. Each function takes different streamer.
What about this one?
```
private void streamOrThrow(final XLogStreamer streamer, final String jobId,
final Writer writer) throws IOException {
try {
streamJobLog(streamer, jobId, writer);
} catch (final CommandException ce) {
throw new IOException(ce);
}
}
...
streamOrThrow(new XLogErrorStreamer(requestParameters), jobId, writer);
...
streamOrThrow(new XLogAuditStreamer(requestParameters), jobId, writer);
```
> On March 9, 2017, 1:36 p.m., András Piros wrote:
> > core/src/main/java/org/apache/oozie/util/XLogStreamer.java
> > Lines 318-327 (patched)
> > <https://reviews.apache.org/r/57107/diff/1/?file=1659391#file1659391line321>
> >
> > Parameter `writtenBytes` is not used. Is it intentional? If so, please
> > rename it, like `byteCountIgnored`.
> >
> > Moreover, please rename the method to e.g. `checkAndFlush()` to better
> > reflect what it actually does.
>
> Purshotam Shah wrote:
> writtenBytes is used. checkAndFlush may not be an appropriate name
> because the function doesn't call flush. It only says should caller flush or
> not.
You're right in that `writtenBytes` is used. But you really should be aware of
that the field `writtenDataSize` is also modified apart from doing a check,
that's why I suggested another name that reflects the fact that the global
state is modified here.
- András
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57107/#review168443
-----------------------------------------------------------
On March 14, 2017, 2:03 a.m., Purshotam Shah wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57107/
> -----------------------------------------------------------
>
> (Updated March 14, 2017, 2:03 a.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-2815
> https://issues.apache.org/jira/browse/OOZIE-2815
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> OOZIE-2815 oozie not always display job log
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/oozie/BaseEngine.java
> 50df8978773ed54fb5a39bb142d7235de1ed396f
> core/src/main/java/org/apache/oozie/BundleEngine.java
> d0099b48772069217375c032d171e8dc6b1bbbbd
> core/src/main/java/org/apache/oozie/CoordinatorEngine.java
> 2f9f8227f1b2bbb0074cfa19aac37ab9a3fe7c0f
> core/src/main/java/org/apache/oozie/DagEngine.java
> 57d276199de149b294f64570a8878b660e9a5a0c
> core/src/main/java/org/apache/oozie/service/XLogService.java
> 04f04f41403fc6446967524124b8cf86f816d71c
> core/src/main/java/org/apache/oozie/service/XLogStreamingService.java
> c15c4c1731838655d8f899f9c0405b226f8111b0
> core/src/main/java/org/apache/oozie/service/ZKXLogStreamingService.java
> 97771ad22be84d1a8167c69a6b48b7012652cf8b
> core/src/main/java/org/apache/oozie/servlet/V0JobServlet.java
> d3b468965f33e300d7ca0f31efc596fd969b47a5
> core/src/main/java/org/apache/oozie/servlet/V1JobServlet.java
> 9356768639e6e3ea548b25baa75d576351fa80e3
> core/src/main/java/org/apache/oozie/util/AuthUrlClient.java
> b45a96acc397c26cdba8d5c62503013dd572f247
> core/src/main/java/org/apache/oozie/util/TimestampedMessageParser.java
> a676f4d35a49147aadde73357397b211ab829c85
> core/src/main/java/org/apache/oozie/util/XLogAuditFilter.java
> c377db5df5dd0bbeed77b4d5c88e1c4e2e64a51e
> core/src/main/java/org/apache/oozie/util/XLogAuditStreamer.java
> e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/util/XLogErrorStreamer.java
> e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/util/XLogFilter.java
> 3b49f77e016cbeaedfa03708dbc680e68f40ca16
> core/src/main/java/org/apache/oozie/util/XLogStreamer.java
> 19f1fee55ba8d6c456e798f01e55cfe84cfb5803
> core/src/main/resources/oozie-default.xml
> 95e0c36e1b2f5aef18637f6f956a990ba36166f2
> core/src/test/java/org/apache/oozie/TestCoordinatorEngineStreamLog.java
> 3eb1016f3ffccaa9db077580b90cc1b3c53fd479
> core/src/test/java/org/apache/oozie/service/TestConfigurationService.java
> 42ffdbeb41e5829cdf814cf13ada331973fc33bd
> core/src/test/java/org/apache/oozie/service/TestXLogStreamingService.java
> bebb678798ec4a01c9710660d4b99fddd5167560
> core/src/test/java/org/apache/oozie/service/TestZKXLogStreamingService.java
> fca8d844f1563da5856eff36f53f21d519630d31
> core/src/test/java/org/apache/oozie/util/TestLogStreamer.java
> f90f784503e101ae2f96fee52977ee83f3098301
>
> core/src/test/java/org/apache/oozie/util/TestSimplifiedTimestampedMessageParser.java
> ea899faf00aa2f98f43838987c9b716321a50dea
> core/src/test/java/org/apache/oozie/util/TestTimestampedMessageParser.java
> 9e28cbc77129bb2675b92ec9c18c77accfd78a55
> core/src/test/java/org/apache/oozie/util/TestXLogUserFilterParam.java
> 46f273fde7f14b446463448c90f22519761997b6
> webapp/src/main/webapp/oozie-console.js
> 76864a9a06e98357c2d17e35609e2a76afbd595c
>
>
> Diff: https://reviews.apache.org/r/57107/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Purshotam Shah
>
>
