issue: make_subscriber() doesn't make a subscriber

[anguillanneuf]

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

	def make_subscriber(
	subscription: SubscriptionPath,
	per_partition_flow_control_settings: FlowControlSettings,
	callback: MessageCallback,
	nack_handler: Optional[NackHandler] = None,
	message_transformer: Optional[MessageTransformer] = None,
	fixed_partitions: Optional[Set[Partition]] = None,
	executor: Optional[ThreadPoolExecutor] = None,
	credentials: Optional[Credentials] = None,
	client_options: Optional[ClientOptions] = None,
	metadata: Optional[Mapping[str, str]] = None,
	) -> StreamingPullFuture:

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) 

[dpcollins-google]

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?

[anguillanneuf]

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?

[dpcollins-google]

No, it uses

python-pubsublite/google/cloud/pubsublite_v1/services/subscriber_service/async_client.py

Line 47 in ec19dfc

class SubscriberServiceAsyncClient:

. There's no reason IMO to expose the underlying protocol methods in the implementation.

[anguillanneuf]

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.

[dpcollins-google]

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.

[anguillanneuf]

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.

[anguillanneuf]

Fixed via #56