You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flume.apache.org by Jarek Cecho <ja...@apache.org> on 2012/07/04 16:23:48 UTC
Review Request: FLUME-1338: Produce helpful error message in case that
timestamp header is missing when time based bucketing is in use
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5746/
-----------------------------------------------------------
Review request for Flume.
Description
-------
I've added catch block that will repack NumberFormatException into RuntimeException with helpful error message.
This addresses bug FLUME-1338.
https://issues.apache.org/jira/browse/FLUME-1338
Diffs
-----
/trunk/flume-ng-core/src/main/java/org/apache/flume/formatter/output/BucketPath.java 1357243
Diff: https://reviews.apache.org/r/5746/diff/
Testing
-------
mvn test
Thanks,
Jarek Cecho
Re: Review Request: FLUME-1338: Produce helpful error message in case that
timestamp header is missing when time based bucketing is in use
Posted by Hari Shreedharan <hs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5746/#review8876
-----------------------------------------------------------
/trunk/flume-ng-core/src/main/java/org/apache/flume/formatter/output/BucketPath.java
<https://reviews.apache.org/r/5746/#comment18911>
We could just move the Long.valueOf call here, thus needing only one try-catch-throw.
/trunk/flume-ng-core/src/main/java/org/apache/flume/formatter/output/BucketPath.java
<https://reviews.apache.org/r/5746/#comment18910>
Shouldn't the exception be caught here as well?
I think a better approach is to check where if timestampHeader is null/non-numeric value. If it is null, you can throw the exception there itself, rather than having multiple checks later.
- Hari Shreedharan
On July 4, 2012, 2:23 p.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5746/
> -----------------------------------------------------------
>
> (Updated July 4, 2012, 2:23 p.m.)
>
>
> Review request for Flume.
>
>
> Description
> -------
>
> I've added catch block that will repack NumberFormatException into RuntimeException with helpful error message.
>
>
> This addresses bug FLUME-1338.
> https://issues.apache.org/jira/browse/FLUME-1338
>
>
> Diffs
> -----
>
> /trunk/flume-ng-core/src/main/java/org/apache/flume/formatter/output/BucketPath.java 1357243
>
> Diff: https://reviews.apache.org/r/5746/diff/
>
>
> Testing
> -------
>
> mvn test
>
>
> Thanks,
>
> Jarek Cecho
>
>
Re: Review Request: FLUME-1338: Produce helpful error message in case that
timestamp header is missing when time based bucketing is in use
Posted by Hari Shreedharan <hs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5746/#review8879
-----------------------------------------------------------
Ship it!
Ship It!
- Hari Shreedharan
On July 5, 2012, 6:38 a.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5746/
> -----------------------------------------------------------
>
> (Updated July 5, 2012, 6:38 a.m.)
>
>
> Review request for Flume.
>
>
> Description
> -------
>
> I've added catch block that will repack NumberFormatException into RuntimeException with helpful error message.
>
>
> This addresses bug FLUME-1338.
> https://issues.apache.org/jira/browse/FLUME-1338
>
>
> Diffs
> -----
>
> /trunk/flume-ng-core/src/main/java/org/apache/flume/formatter/output/BucketPath.java 1357243
>
> Diff: https://reviews.apache.org/r/5746/diff/
>
>
> Testing
> -------
>
> mvn test
>
>
> Thanks,
>
> Jarek Cecho
>
>
Re: Review Request: FLUME-1338: Produce helpful error message in case that
timestamp header is missing when time based bucketing is in use
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5746/
-----------------------------------------------------------
(Updated July 5, 2012, 6:38 a.m.)
Review request for Flume.
Changes
-------
Hi Hari,
thank you very much for your review. You're definitely right that the try-catch block should have been on both places. I've moved some code around to avoid having same code on multiple places.
Jarcec
Description
-------
I've added catch block that will repack NumberFormatException into RuntimeException with helpful error message.
This addresses bug FLUME-1338.
https://issues.apache.org/jira/browse/FLUME-1338
Diffs (updated)
-----
/trunk/flume-ng-core/src/main/java/org/apache/flume/formatter/output/BucketPath.java 1357243
Diff: https://reviews.apache.org/r/5746/diff/
Testing
-------
mvn test
Thanks,
Jarek Cecho