-
Notifications
You must be signed in to change notification settings - Fork 1k
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
feat: support for tunable retention, grace period for windowed tables #4733
feat: support for tunable retention, grace period for windowed tables #4733
Conversation
@confluentinc It looks like @vinothchandar just signed our Contributor License Agreement. 👍 Always at your service, clabot |
bc6bc8d
to
810072c
Compare
091043e
to
65cde48
Compare
- Fixes confluentinc#4157, adding SQL syntax for specifying retention, grace period - Added functional tests feat: initial commit for tunable retention feat: hacky test now works
82dd65f
to
73c459c
Compare
Tests do pass locally.. On jenkins hitting some weird failures. Looking into them |
@JimGalasyn could you review the doc changes? |
77276c8
to
07035eb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, with a few suggestions.
07035eb
to
1f367ac
Compare
Thanks @JimGalasyn ! Updated with the suggestions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for fixing this outstanding issue
ksqldb-execution/src/main/java/io/confluent/ksql/execution/windows/WindowTimeClause.java
Show resolved
Hide resolved
...db-streams/src/test/java/io/confluent/ksql/execution/streams/StreamAggregateBuilderTest.java
Show resolved
Hide resolved
1f367ac
to
febc520
Compare
Description
Both clauses are optional, and will fallback to Kafka streams defaults if unspecified.
Testing done
Reviewer checklist