Skip to content

add durable sse subscriptions #20

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

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

jkarneges
Copy link
Member

This enables SSE clients to establish durable subscriptions, by including query parameter durable=true. When durability is enabled, the any messages retained in KV Store are replayed upon establishment.

The IDs used in SSE events are a concatenation of the topics of interest and their last known retained versions, e.g.:

event: message
id: jtest:d06fe8f6d007c421-8,jtest2:a7b89db793b3da37-5
data: other msg 1

event: message
id: jtest:d06fe8f6d007c421-8,jtest2:a7b89db793b3da37-6
data: other msg 2

If a client reconnects and specifies a received event ID in the Last-Event-ID request header, the server can avoid replaying messages the client has already seen.

Messages are delivered by publishing hint actions to Fanout which trigger message fetching. This avoids the need to personalize the event IDs in published payloads. Later on, we can look at implementing such personalization to avoid fetching.

Comment on lines +189 to +190
let topic = &part[..pos];
let version = &part[(pos + 1)..];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should add some safeguards around this indexing and throw a bad-request error if it fails?


// close
return Response::new();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be my misunderstanding but if we hit this case here should we return a 200?

Comment on lines +177 to +181
} else if let Some(s) = req.get_header_str("Last-Event-ID") {
Some(s)
} else {
None
};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is just .map(|s| s)

else {
     req.get_header_str("Last-Event-ID").map(|s| s)
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants