Enables property for setting AT_TIMESTAMP shard iterator initial time… #342
Conversation
| @@ -0,0 +1,49 @@ | |||
| /* | |||
| * Copyright 2014 Amazon.com, Inc. or its affiliates. All Rights Reserved. | |||
There was a problem hiding this comment.
Can you update this to 2018?
|
Could you also add a basic unit test? Thanks |
|
Hi @pfifer. Fixup commit added with update header and tests. |
|
Hi @pfifer. Please let me know if there is anything else I should address in this PR. Otherwise, if you're happy with the change, it would be great to get it merged. Thanks. |
| assertEquals(timestamp, new Date(Long.parseLong(TEST_VALUE) * 1000L)); | ||
| } | ||
|
|
||
| @Test |
There was a problem hiding this comment.
Minor: You can do
@Test(expected = IllegalArgumentException.class)`which would remove the requirement for the try/catch.
| @Override | ||
| public Date decodeValue(String value) { | ||
| try { | ||
| return new Date(Long.parseLong(value) * 1000L); |
There was a problem hiding this comment.
Just calling this out for myself, and any future readers:
If someone accidentally uses milliseconds this won't work the way they expect. In the future it might make sense to use Instant from java.time. Instant#parse can deal with textual date representations, and can be converted to a java.util.Date pretty easily.
| "streamName = a", | ||
| "applicationName = b", | ||
| "AWSCredentialsProvider = ABCD," + credentialName1, | ||
| "timestampAtInitialPositionInStream = 123abc" |
There was a problem hiding this comment.
Calling this out for myself, and future readers:
Looking at the code this will at least log a warning. That isn't part of your code, but I don't know if that is a good outcome. If the InitialPositionInStream is set to AT_TIMESTAMP then the KCL will be unable to startup if the lease table doesn't exist or isn't populated. Otherwise the KCL should be able to startup, but that could be really confusing.
|
Thanks for providing the change. I'm going to start the process for accepting this on my side. For my own information which of the MultiLang clients do you use? |
…r initial timestamp (awslabs#341)
I'm using ruby client for the time being. |
…stamp (#341)
Issue #341
Description of changes:
Added
DatePropertyValueDecoderfor use inKinesisClientLibConfiguratorwhich enablestimestampAtInitialPositionInStreamproperty for setting initial timestamp forAT_TIMESTAMPshard iterator.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.