-
Notifications
You must be signed in to change notification settings - Fork 105
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
subscription interface and event streaming for low latency push events #508
Conversation
a94fb25
to
12601c3
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.
Just left a few small nits, and a comment about signing the event log messages.
// }, | ||
// authorizationSignatureInput : Jws.createSignatureInput(tennat), | ||
// }) | ||
// this.config.eventStream.add(logMessage) |
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.
Sanity: I don't think we're not able to sign here because EventLog.append()
is called from within the handler after the message has already been signed and received by the DWN.
But I think you're right, not sure we need to sign this at all.
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.
yea...i fixed this by just forwarding the message itself for now.
src/interfaces/event-create.ts
Outdated
|
||
export type EventCreateOptions = { | ||
descriptor: EventDescriptor; | ||
messageId?: string; |
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.
messageId?: string; | |
messageCid?: string; |
src/interfaces/event-create.ts
Outdated
interface : DwnInterfaceName.Subscriptions, | ||
method : DwnMethodName.Request, | ||
messageTimestamp : options.messageTimestamp ?? getCurrentTimeInHighPrecision(), | ||
messageId : options.messageId, |
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.
messageId : options.messageId, | |
messageCid : options. messageCid, |
method: DwnMethodName.Request; | ||
messageTimestamp: string; | ||
messageId?: string; // attached message |
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.
messageId?: string; // attached message | |
messageCid?: string; // attached message |
Closing, this work has been migrated to: #607 |
TP here: https://hackmd.io/vrC1iCudTVuM8nryo3QQKA#Subscribing-to-your-own-Node
A few things I want to clarify here:
SubscriptionRequest returns an EventEmitter, which is what a dwn will listen to for streaming events.
Note: the
dwn-sdk-js
is not in charge of actually managing the connection between Alice and Bob. Alice is simply getting an "event hook" to listen into that is scoped to a subscription request. She then can choose to send it or not.I have an update to
dwn-server
to handle the web socket connections coming.State: Initial feedback would be helpful, but not ready for merging yet.