Add support for custom offset retention durations to offset manager #602
Conversation
|
Thanks! I think this kind of value makes more sense in the |
|
Ok changed. Can you take another look? |
|
Test this please |
|
I'm a human being who has to eat and sleep just like everyone else, please be patient. |
| @@ -228,6 +228,33 @@ func TestPartitionOffsetManagerMarkOffset(t *testing.T) { | |||
| coordinator.Close() | |||
| } | |||
|
|
|||
| func TestPartitionOffsetManagerMarkOffsetWithRetention(t *testing.T) { | |||
There was a problem hiding this comment.
This test doesn't actually test that the value in the request is set correctly. It should do that, or it (the test) should be removed.
There was a problem hiding this comment.
Instead of a sleep and a coordinator.Returns you should be able to call coordinator.setHandler and validate the request values in the callback.
There was a problem hiding this comment.
Ok changed, that does look a lot better.
|
Sorry that was meant for Travis-ci actually :). The check failed and I thought I could get travis-ci to re-test it with "Test this please" (That is how you force jenkins to re-check, thought Travis-ci is the same.). Didn't mean that to rush you. |
| @@ -183,6 +183,10 @@ type Config struct { | |||
| // The initial offset to use if no offset was previously committed. | |||
| // Should be OffsetNewest or OffsetOldest. Defaults to OffsetNewest. | |||
| Initial int64 | |||
|
|
|||
| // The retention duration for committed offsets. | |||
| // A value of 0 cause the retention time to default to the offsets.retention.minutes broker option. | |||
There was a problem hiding this comment.
Please follow the style of other option fields:
- keep lines short enough to display properly on godoc.org
- explicitly mention the default value
- mention that Kafka only supports precision up to milliseconds; nanoseconds will be truncated.
- add a warning to the
Validatemethod if greater-than-millisecond precision is provided
|
Oh, heh I had no idea Jenkins worked like that, Travis doesn't :) I think you can trigger a rebuild from the Travis page that github links to. |
|
Thanks for your work! |
Add support for custom offset retention durations to offset manager
No description provided.