-
Notifications
You must be signed in to change notification settings - Fork 183
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Take with timestamp #242
Take with timestamp #242
Conversation
@patkivikram and @ekerstens I saw that you are pretty active, could you have a look? 🙏 |
The code looks clean to me, but I think for a user facing function, there really needs to be a unit test of some kind for it. |
@taybin could you have a look again? I added 3 tests, should cover the most essentials parts. |
Codecov Report
@@ Coverage Diff @@
## master #242 +/- ##
==========================================
- Coverage 94.32% 94.26% -0.06%
==========================================
Files 100 100
Lines 10762 10814 +52
Branches 1472 1477 +5
==========================================
+ Hits 10151 10194 +43
- Misses 542 549 +7
- Partials 69 71 +2
Continue to review full report at Codecov.
|
@patkivikram if you have some time to look into that would be amazing, it's pretty trivial. Many thanks. |
Add take_with_timestamp
Description
I'm currently using the take method of a stream in production. I have some cases where I would like to have not only the batch of values but a batch of values with the timestamp of when the message was inserted into kafka.
This PR adds a new method called take_with_timestamp, it works exactly like take, but it adds the Kafka timestamp to each returned value.
I tested in a local environment and seems working as expected, the real difference with take is this:
the name of the timestamp must be passed as an argument, to force to pass the name, some people might like kafka_timestamp, some event_timestamp, I preferred to leave it generic.