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

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

Merged
merged 8 commits into from Nov 5, 2020

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.

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