You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flume.apache.org by Umesh Chaudhary <um...@gmail.com> on 2016/09/23 12:22:00 UTC
Review Request 52216: Patch for FLUME-2647
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52216/
-----------------------------------------------------------
Review request for Flume.
Repository: flume-git
Description
-------
Added two new booleans to identify the reason for closed channel: 1) isClosedOnStart 2) isClosedNormally
And checked them to print the appropriate reason for closed channel
Diffs
-----
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java 9d82e43
Diff: https://reviews.apache.org/r/52216/diff/
Testing
-------
Yes
Thanks,
Umesh Chaudhary
Re: Review Request 52216: Patch for FLUME-2647
Posted by Balázs Donát Bessenyei <be...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52216/#review150298
-----------------------------------------------------------
Do you think you could create a unit test for this fix?
Like triggering an error and checking if the message is correct
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java (line 107)
<https://reviews.apache.org/r/52216/#comment218203>
This field seems to be never accessed.
Maybe it should be removed.
- Bal�zs Don�t Bessenyei
On Sept. 24, 2016, 9:54 a.m., Umesh Chaudhary wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52216/
> -----------------------------------------------------------
>
> (Updated Sept. 24, 2016, 9:54 a.m.)
>
>
> Review request for Flume.
>
>
> Repository: flume-git
>
>
> Description
> -------
>
> Added two new booleans to identify the reason for closed channel: 1) isClosedOnStart 2) isClosedNormally
> And checked them to print the appropriate reason for closed channel
>
>
> Diffs
> -----
>
> flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java 9d82e43
>
> Diff: https://reviews.apache.org/r/52216/diff/
>
>
> Testing
> -------
>
> Yes
>
>
> Thanks,
>
> Umesh Chaudhary
>
>
Re: Review Request 52216: Patch for FLUME-2647
Posted by Umesh Chaudhary <um...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52216/
-----------------------------------------------------------
(Updated Sept. 25, 2016, 10 a.m.)
Review request for Flume.
Changes
-------
Applied Lior's review comments
Repository: flume-git
Description (updated)
-------
Added a new booleans to identify the reason for closed channel: isClosedNormally
And checked them to print the appropriate reason for closed channel
Diffs (updated)
-----
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java 9d82e43
flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestFileChannel.java bfc2d0d
Diff: https://reviews.apache.org/r/52216/diff/
Testing
-------
Yes
Thanks,
Umesh Chaudhary
Re: Review Request 52216: Patch for FLUME-2647
Posted by Lior Zeno <li...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52216/#review150299
-----------------------------------------------------------
Fix it, then Ship it!
flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestFileChannel.java (line 650)
<https://reviews.apache.org/r/52216/#comment218204>
style nit: can you please join the first two strings?
flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestFileChannel.java (line 656)
<https://reviews.apache.org/r/52216/#comment218205>
same here
- Lior Zeno
On Sept. 24, 2016, 12:23 p.m., Umesh Chaudhary wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52216/
> -----------------------------------------------------------
>
> (Updated Sept. 24, 2016, 12:23 p.m.)
>
>
> Review request for Flume.
>
>
> Repository: flume-git
>
>
> Description
> -------
>
> Added two new booleans to identify the reason for closed channel: 1) isClosedOnStart 2) isClosedNormally
> And checked them to print the appropriate reason for closed channel
>
>
> Diffs
> -----
>
> flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java 9d82e43
> flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestFileChannel.java bfc2d0d
>
> Diff: https://reviews.apache.org/r/52216/diff/
>
>
> Testing
> -------
>
> Yes
>
>
> Thanks,
>
> Umesh Chaudhary
>
>
Re: Review Request 52216: Patch for FLUME-2647
Posted by Umesh Chaudhary <um...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52216/
-----------------------------------------------------------
(Updated Sept. 24, 2016, 12:23 p.m.)
Review request for Flume.
Changes
-------
Incorporated Bal�zs's comments and added unit tests.
Repository: flume-git
Description
-------
Added two new booleans to identify the reason for closed channel: 1) isClosedOnStart 2) isClosedNormally
And checked them to print the appropriate reason for closed channel
Diffs (updated)
-----
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java 9d82e43
flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestFileChannel.java bfc2d0d
Diff: https://reviews.apache.org/r/52216/diff/
Testing
-------
Yes
Thanks,
Umesh Chaudhary
Re: Review Request 52216: Patch for FLUME-2647
Posted by Umesh Chaudhary <um...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52216/
-----------------------------------------------------------
(Updated Sept. 24, 2016, 9:54 a.m.)
Review request for Flume.
Changes
-------
Incorporated Jeff's comments.
Repository: flume-git
Description
-------
Added two new booleans to identify the reason for closed channel: 1) isClosedOnStart 2) isClosedNormally
And checked them to print the appropriate reason for closed channel
Diffs (updated)
-----
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java 9d82e43
Diff: https://reviews.apache.org/r/52216/diff/
Testing
-------
Yes
Thanks,
Umesh Chaudhary
Re: Review Request 52216: Patch for FLUME-2647
Posted by Jeff Holoman <je...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52216/#review150251
-----------------------------------------------------------
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java (line 379)
<https://reviews.apache.org/r/52216/#comment218140>
Do we need to add "because it was closed normally"? It seems like if it's normal we just say "Channel Closed" and if not, you get the error message
- Jeff Holoman
On Sept. 23, 2016, 12:22 p.m., Umesh Chaudhary wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52216/
> -----------------------------------------------------------
>
> (Updated Sept. 23, 2016, 12:22 p.m.)
>
>
> Review request for Flume.
>
>
> Repository: flume-git
>
>
> Description
> -------
>
> Added two new booleans to identify the reason for closed channel: 1) isClosedOnStart 2) isClosedNormally
> And checked them to print the appropriate reason for closed channel
>
>
> Diffs
> -----
>
> flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java 9d82e43
>
> Diff: https://reviews.apache.org/r/52216/diff/
>
>
> Testing
> -------
>
> Yes
>
>
> Thanks,
>
> Umesh Chaudhary
>
>