-
Notifications
You must be signed in to change notification settings - Fork 10
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: implement sdk for session reduce #94
Merged
Merged
Changes from 1 commit
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
a84e932
add user interfaces
KeranYang 16c0571
copy reduce streamer
KeranYang 12cd9b8
add proto - coverage to fix
KeranYang d908719
use new proto - coverage to fix
KeranYang 25950d3
open-append-close
KeranYang 518bab6
open-expand-close
KeranYang 92bb0e9
fix unit tests
KeranYang 95a03ae
implement merge operation and add unit tests
KeranYang 717f699
update comments
KeranYang 9e15827
add more error tests and make server test same as golang
KeranYang 4a9c7eb
fix merge into an existing window
KeranYang 7399ea3
fix merge and close race condition
KeranYang a481c53
properly handle EOF
KeranYang bf22f7f
add more tests
KeranYang 18c4151
unify actor responses
KeranYang 1e06d61
introduce builder pattern
KeranYang 31c7e1f
clean up logs
KeranYang 720c0db
clean up
KeranYang 144a380
add an example
KeranYang 7b56abe
fix: cover the case when EOF received but no active window
KeranYang 7859b59
.
KeranYang ff17ebd
add 3 failed test cases - pending fix
KeranYang a85201e
Merge branch 'main' into session
KeranYang 8cc3eff
fix merge conflict
KeranYang cf4efa5
make MERGE a blocking call
KeranYang 953c623
address comments
KeranYang File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
7 changes: 7 additions & 0 deletions
7
src/main/java/io/numaproj/numaflow/sessionreducer/MergeDoneResponse.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
package io.numaproj.numaflow.sessionreducer; | ||
|
||
/** | ||
* MergeDoneResponse indicates the current MERGE request is done. | ||
*/ | ||
public class MergeDoneResponse { | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Hacky, sometimes it might take more than 1 second
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.
thanks, what should be the proper number? @yhl25
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.
let's wait until it returns
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.
also, please add unit tests to cover merge followed by another operation to make sure the blocking call works!
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.
The method Await asks for a timeout, I don't think we should wait forever, right? Maybe we should set an atMost timeout value and throws when the operation takes too long.
Yes, the uts for merge are already in place. Please check the serverTest class.