Navigation Menu

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

feat(pubsublite): Retryable stream wrapper #3068

Merged
merged 10 commits into from Oct 27, 2020
Merged

Conversation

tmdiep
Copy link
Contributor

@tmdiep tmdiep commented Oct 22, 2020

This will be used to manage all of Pub/Sub Lite's bidi streaming RPCs: publish, subscribe, streaming cursor commit, partition assignment, etc.

@tmdiep tmdiep requested a review from a team as a code owner October 22, 2020 07:47
@tmdiep tmdiep requested a review from hongalex October 22, 2020 07:48
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Oct 22, 2020
@tmdiep tmdiep changed the title feat(pubsublite) Retryable stream wrapper feat(pubsublite): Retryable stream wrapper Oct 22, 2020
@tmdiep
Copy link
Contributor Author

tmdiep commented Oct 22, 2020

This is loosely based on https://github.com/googleapis/google-cloud-go/blob/master/pubsub/pullstream.go.

An example of usage will be the publisher (not ready for review yet):
https://github.com/tmdiep/google-cloud-go/blob/working/pubsublite/publisher_internal.go

I will consolidate unit tests for the publisher and retryableStream, as there is much overlap.

@product-auto-label product-auto-label bot added the api: pubsublite Issues related to the Pub/Sub Lite API. label Oct 22, 2020
Copy link
Member

@codyoss codyoss left a comment

Choose a reason for hiding this comment

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

I think it would be valuable to add some tests here to ensure no deadlocks/races.

pubsublite/rpc_util.go Outdated Show resolved Hide resolved
pubsublite/streams.go Show resolved Hide resolved
pubsublite/streams.go Outdated Show resolved Hide resolved
pubsublite/streams.go Show resolved Hide resolved
@tmdiep
Copy link
Contributor Author

tmdiep commented Oct 22, 2020

Thanks Cody.

Yes, definitely need tests! Since retryableStream needs a concrete client stream to operate on, there will be substantial overlap with [unit tests](https://github.com/tmdiep/google-cloud-go/blob/working/pubsublite/publisher_internal_test.go] for the upcoming publisher.

@tmdiep tmdiep requested a review from codyoss October 22, 2020 22:52
@tmdiep
Copy link
Contributor Author

tmdiep commented Oct 27, 2020

Thanks everyone for reviewing. Merging..

@tmdiep tmdiep merged commit 97cfd45 into googleapis:master Oct 27, 2020
@tmdiep tmdiep deleted the streams branch October 27, 2020 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: pubsublite Issues related to the Pub/Sub Lite API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants