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 googleapis/yoshi-go-admins as a code owner Oct 22, 2020
@tmdiep tmdiep requested a review from hongalex Oct 22, 2020
@google-cla google-cla bot added the cla: yes label Oct 22, 2020
@tmdiep tmdiep changed the title feat(pubsublite) Retryable stream wrapper feat(pubsublite): Retryable stream wrapper Oct 22, 2020
@tmdiep tmdiep force-pushed the tmdiep:streams branch from 46b5587 to e7db971 Oct 22, 2020
@tmdiep
Copy link
Contributor Author

@tmdiep 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.

Copy link
Member

@codyoss codyoss left a comment

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 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 Oct 22, 2020
@tmdiep tmdiep force-pushed the tmdiep:streams branch from 52f1ae2 to 3989170 Oct 27, 2020
tmdiep and others added 2 commits Oct 27, 2020
@tmdiep
Copy link
Contributor Author

@tmdiep tmdiep commented Oct 27, 2020

Thanks everyone for reviewing. Merging..

@tmdiep tmdiep merged commit 97cfd45 into googleapis:master Oct 27, 2020
3 checks passed
3 checks passed
@google-cla
cla/google All necessary CLAs are signed
@conventional-commit-lint-gcf
conventionalcommits.org
Details
@kokoro-team
kokoro Kokoro CI build successful
Details
@tmdiep tmdiep deleted the tmdiep:streams branch Oct 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants