Skip to content
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

subscribe #210

Merged
merged 10 commits into from
Sep 12, 2024
Merged

subscribe #210

merged 10 commits into from
Sep 12, 2024

Conversation

jchris
Copy link
Contributor

@jchris jchris commented Sep 11, 2024

to support gateway events

if (remote && opts.gateway.subscribe) {
this.logger.Debug().Str("url", url.toString()).Msg("Subscribing to the gateway with URL");
opts.gateway.subscribe(url, (byteHead: Uint8Array) => this.handleEventByteHead(byteHead));
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this subscribe is only called on remote blockstores... this could be implemented as a subclass thing on the meta or something, but I think the way I have it is good enough for testing the runtime.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is a single event, not the multi-event thing that comes from the GET. that's not essential, we could wrap this as a multi if easier

Copy link
Contributor

@mschoch mschoch left a comment

Choose a reason for hiding this comment

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

Looks good to me, two minor questions.

Should gateways expect to only have one subscriber, or should they support multiple invocations of this method. If we only expect 1 for the life of the gateway, it might be cleaner if it was passed in with start()

Did this change break the test you disabled?

@jchris
Copy link
Contributor Author

jchris commented Sep 12, 2024

i've modified the pk gateway to connect on subscribe, not start, so this only connects when used as a remote store

@jchris jchris merged commit d40ca0b into main Sep 12, 2024
2 checks passed
@jchris jchris deleted the gateway-subscribe branch September 20, 2024 20:03
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