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

Pub/Sub streaming pull requests #58

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

SolarLiner
Copy link

@SolarLiner SolarLiner commented Jul 20, 2021

This PR implements endpoints for the Pub/Sub bi-directional stream, with a few limitations at the moment:

  • ACKing messages is done within the API and not exposed to the user for now, I haven't found a good way to expose it in an idiomatic manner to the user yet
  • The streaming method returns a future stream using futures::stream::unfold which does not leave room for proper error handling (the stream can send error messages, always terminating the connection AFAIK), and changing the item type to be Result seems a bit overkill to me. But right now, the stream can fail silently, abruptly finishing the stream.

I had to update the protobuf definitions as well, and I moved to a submodule instead to easily keep them up to date here. I'll revert if that's unwanted.

I needed this feature as part of a high-throughput system, and as such this PR is being battle-tested right now! 😄

closes #57

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.

Pub/Sub streaming pull method
1 participant