issue: make_subscriber() doesn't make a subscriber
anguillanneuf opened this issue · 7 comments
make_subscriber() makes a StreamingPullFuture (while make_async_subscriber() seems like something users are expected to use):
python-pubsublite/google/cloud/pubsublite/cloudpubsub/make_subscriber.py
Lines 205 to 216 in ec19dfc
Current:
streaming_pull_future = make_subscriber(
subscription_path,
per_partition_flow_control_settings=flow_control_settings,
callback=callback,
)
streaming_pull_future.result(timeout=timeout) Expected:
streaming_pull_future = subscriber_client.subscribe(
subscription_path,
per_partition_flow_control_settings=flow_control_settings,
callback=callback,
)
streaming_pull_future.result(timeout=timeout) This assumes that you already have access to a 'subscriber client' object that can make subscribers. But that client has no lifetime concerns and no member variables to the extent that I can tell. Can you provide me the interface of the "SubscriberClient" object that you would want?
Pub/Sub's subscriber client wraps google/pubsub_v1/services/subscriber/client.py and overwrites some methods.
Does the Lite library work with /google/cloud/pubsublite_v1/services/subscriber_service/client.py?
No, it uses
. There's no reason IMO to expose the underlying protocol methods in the implementation.Oh tricky. I don't know if I agree with not exposing the methods in SubscriberServiceAsyncClient. Does other library owners have an opinion? @googleapis/yoshi-python
I would imagine the subscriber client interface to be similar to SubscriberServiceAsyncClient, with optional flow control settings and partitions (default to auto assigner), and it to have a subscribe method that returns a streaming pull future.
Oh tricky. I don't know if I agree with not exposing the methods in SubscriberServiceAsyncClient
The only method on SubscriberServiceAsyncClient is a subscribe async method that returns an AsyncIterator. There's nothing else there other than:
- DEFAULT_ENDPOINT / DEFAULT_MTLS_ENDPOINT which are incorrect for Pub/Sub Lite
- from_service_account_file / from_service_account_json which would need different signatures as they don't provide enough information to connect
- get_transport_class which, tbh, looks like a leaked implementation detail
Which then comes back to the question I asked in my first comment. Do you want a Client class that is just a wrapper of a bag of parameters? Specifically credentials, transport and client options? I can do that, it just seems strange to me.
I'll provide a mock-up of what this would look like.
What I don't understand is why the imperfections in the listed methods (credentials, transport, and client options) can't be overwritten and fixed in the handwritten layer. I recently approved a fix in the handwritten layer by a MTLS folk (googleapis/python-pubsub#226).
I expect users to set flow control settings and partitions on the subscriber, then start subscribing similar to how it's done in Java:
Subscriber subscriber = Subscriber.create(subscriberSettings);
subscriber.startAsync().awaitRunning();Our public docs introduce subscriber as a concept when explaining how Pub/Sub Lite works. Not having a subscriber client is going to be confusing.
Fixed via #56