Introduce suppress to streams.
#23
Conversation
|
Hey - thanks for this Daniel. Regards |
|
Hey, @cddr. It's been a while. I've updated the PR and added a lot more test cases. Let me know what you think. Would be great to get a new version with including this out, once it's in a state that works for you. Best |
Codecov Report
@@ Coverage Diff @@
## master #23 +/- ##
=========================================
+ Coverage 81.43% 81.6% +0.17%
=========================================
Files 40 39 -1
Lines 2391 2386 -5
Branches 149 151 +2
=========================================
Hits 1947 1947
+ Misses 295 288 -7
- Partials 149 151 +2
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #23 +/- ##
==========================================
+ Coverage 81.62% 81.64% +0.01%
==========================================
Files 39 39
Lines 2373 2386 +13
Branches 149 151 +2
==========================================
+ Hits 1937 1948 +11
Misses 287 287
- Partials 149 151 +2
Continue to review full report at Codecov.
|
|
Cool, thanks @cddr. Anything I still should do/help do to get it merged? Some circleci job fails, but that does not seem to be test related. |
| be unbounded. If `until-time-limit-ms` is set, this will override the | ||
| `TimeWindow` interval. Note that when relying on the configured `TimeWindow` | ||
| the default `grace` period is `24h - window-size`.") | ||
|
|
There was a problem hiding this comment.
could be helpful to format the docstring after the styleguide. just ending the first line after the first sentence and starting "You can either specify..." on a newline
https://github.com/bbatsov/clojure-style-guide#docstring-summary
src/jackdaw/streams/interop.clj
Outdated
| (suppress | ||
| [_ suppress] | ||
| (clj-ktable | ||
| (.suppress ktable (suppress-config suppress)))) |
There was a problem hiding this comment.
it looks like the spec defined before was named suppress-config but the fn that uses it is called suppress-config and the config is named suppress. this seems a little confusing to me.
might be helpful to name the config arg suppress-config to match the spec and the fn create-suppress-config or something. the names i suggested could definitely be better though
There was a problem hiding this comment.
I've tried to make more sense of the names. The argument is now suppress-config and the function is suppress-config->suppressed which is what it's doing actually: turning a suppress-config into a org.apache.kafka.streams.kstream.Suppressed.
This adds the `suppress` operator for Kafka Streams apps. This allows to only emit messages for a specific key once within a specific time window, e.g.
|
Hey @vijumathew, could you have another look at this? |
Version 2.1 introduced a new operator: suppress. It allows to only
forward the final result of a windowed aggregation.
This PR adds the
suppressfunction to the streams namespace andallows for the new operator to be used in Kafka Streams apps. It works but may not be in a final stage. Asking for feedback at this point whether this is what you have in mind or I should change something.
Best
Daniel