-
Notifications
You must be signed in to change notification settings - Fork 6
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: implemented submit chunking for RFD #0010 #737
base: main
Are you sure you want to change the base?
Conversation
- Added support for chunking Query submissions - Added test for submission chunking Signed-off-by: Patrick Casey <[email protected]>
e6d0795
to
a11f303
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.
Missing some logic in the QuerySynthesizer, but the rest is looking good!
QueryState::SubmitInProgress | QueryState::SubmitComplete => { | ||
return Err(Error::ReceivedSubmitWhenExpectingReplyChunk) | ||
} |
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.
I believe the QuerySynthesizer
needs to be modified as well to support de-chunking chunked Submit
queries.
// ensure last one is ...Complete | ||
assert_eq!(res.last().unwrap().state(), final_state); | ||
assert_eq!(res.len(), 4); | ||
} |
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.
To ensure the above QuerySynthesizer
is implemented correctly, perhaps there should be an additional stage to this test that tries to reassemble the original message from the fragments.
On the subject of versioning, I would do the following:
We can add RFD10 to On the next release, we will release new versions of the rust SDK and all plugins. |
This is not quite done yet, still working through a couple of things |
I just want to ensure that adding an enum variant to
QueryState
is not considered a breaking change. Also since this changeshipcheck-common
, does its version need be incremented or do we just handle that when it comes time to cut a release