-
Notifications
You must be signed in to change notification settings - Fork 11
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: streaming delta api #751
base: main
Are you sure you want to change the base?
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files |
…leash/unleash-edge into feat/streaming-delta-epi-edge-support
…aming-delta-epi-edge-support
|
||
if tokio::time::timeout(std::time::Duration::from_secs(2), async { | ||
loop { | ||
if let Some(Ok(event)) = stream.next().await { | ||
let next = stream.next().await; | ||
if let Some(Ok(event)) = next { |
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.
do match instead
if tokio::time::timeout(std::time::Duration::from_secs(2), async { | ||
loop { | ||
if let Some(Ok(event)) = stream.next().await { | ||
let next = stream.next().await; | ||
if let Some(Ok(event)) = next { |
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.
match
server/tests/streaming_test.rs
Outdated
assert_eq!(initial_features.query, update.query); | ||
assert_eq!(initial_features.version, update.version); | ||
assert_ne!(initial_features.features, update.features); | ||
// TODO: uncomment when we can filter by id |
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.
can uncomment this one now since we do filtering
server/src/http/broadcaster.rs
Outdated
&self, | ||
query: StreamingQuery, | ||
) -> Result<DeltaEvent, EdgeError> { | ||
// do we need filter_set for hydration event? |
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.
we do need to filter hydration event features. will be done in another PR
About the changes
This PR adds streaming support for delta events.
Important terminology:
On the wire it looks like this:
or
When SDK connects to edge for the first time it gets unleash-connected event with the hydration event type.
On each subsequent update in Unleash only unleash-updated events are propagated with the changes that the SDK hasn't seen yet. For each connection Edge remembers the last event id that has been broadcasted before.
Important files
Discussion points