Skip to content

fix: Make public API more similar to generated clients#56

Merged
dpcollins-google merged 8 commits intomasterfrom
client_attempt_2
Nov 5, 2020
Merged

fix: Make public API more similar to generated clients#56
dpcollins-google merged 8 commits intomasterfrom
client_attempt_2

Conversation

@dpcollins-google
Copy link
Collaborator

@dpcollins-google dpcollins-google commented Oct 27, 2020

Fixes #45

@dpcollins-google dpcollins-google requested a review from a team as a code owner October 27, 2020 18:14
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Oct 27, 2020
@dpcollins-google dpcollins-google added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Oct 27, 2020
Copy link
Collaborator

@anguillanneuf anguillanneuf left a comment

Choose a reason for hiding this comment

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

This largely works. What's left are:

  1. Clean up module imports so AdminClient, PublisherClient, and SubscriberClient can be directly imported from google.cloud.pubsublite.
  2. Fix ValueError: ClientOptions does not accept an option 'x-goog-pubsub-context'. I came across this one while trying out publish.
  3. assert self._close_callback is not None in Line 78 google/cloud/pubsublite/cloudpubsub/internal/subscriber_impl.py failed while I was trying out subscribe.
  4. Add tests.
  5. Add documentation.

I'm not able to publish or subscribe successfully due to these bugs, but if we can get the library working, I think we are good..

@dpcollins-google
Copy link
Collaborator Author

  1. I've made them importable from google.cloud.pubsublite.cloudpubsub, given that these are the Cloud Pub/Sub emulating clients to distinguish them from the GAPIC wire protocol ones already exported in google.cloud.pubsublite.

  2. Done.

  3. Done.

  4. WIP

  5. WIP

@dpcollins-google
Copy link
Collaborator Author

  1. Fixed.
  2. Fixed.

@anguillanneuf
Copy link
Collaborator

Thank you for all the fixes. I will test it out with samples on Monday.

Copy link
Collaborator

@anguillanneuf anguillanneuf left a comment

Choose a reason for hiding this comment

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

Fixes #45

The publisher and subscriber client creation looks good and works in samples.

The remaining issue about suppressing exceptions during subscriber shutdown will be addressed in a later PR.

@dpcollins-google dpcollins-google merged commit 7cf02ae into master Nov 5, 2020
@anguillanneuf anguillanneuf deleted the client_attempt_2 branch March 25, 2022 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement. do not merge Indicates a pull request not ready for merge, due to either quality or timing.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

issue: atypical client creation

2 participants