You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[Pub/Sub] Add a bare Subscriber factory to Client #3369
Current Pub / Sub API requires a topic to create a subscription. This is unnecessary and cumbersome. It is also not feasible for some use cases: some users will only be given a subscription resource name to work with and not know the topic.
@kir-titievsky In our current implementation, a subscription with self.topic set to None will not be able to call Subscription.create() (there may be other methods that require a topic to be set on a Subscription but none that I can tell).
Can you help me understand when a Subscriptionshould have a Topic and when it doesn't matter? Also, can you give an example of a "a subscription resource name to work with and not know the topic"?
@dhermes Thank you for the prompt consideration! Good points. I did not consider the pubsub.Topic() and pubsub.Subscription() factories since they are not used in the documentation examples. We can fix that bit.
That said, this still seems really not super great:
There is an asymmetry here: pubsub.Client().topic() but a subscription but a subscription as pubsub.Client().topic().subscription()?
You could argue that pubsub.Subscription() and pubsub.Topic() are the right way to create these, but both of these factories take a client as an argument. So you still have to do:
Which would have been at least consistent, but we provide a much more comfortable interface for the topic: topic = pubsub.Client().topic() which leads me to expect the same for a subscription and also ask why would there be two separate interfaces for creating a subscription (or a topic) one of which is mildly (the pubsub.Topic(name, client=pubsub.Client()) and the other is inconsistent between Topic and Subscriber. The way this was solved in Java is to have separate data and admin classes (Subscriber and SubscriberAdmin). Perhaps a better thing to do is to say that Client() holds all admin operations, like list_topic() and list_subscription() as well as factories for creating subscribers and topics.
Won't the topic be embedded in the subscription resource name?
True. This is not an issue of keeping the topic name from the user. It's about not requiring the user to know the topic name in order to create a subscription in the first place.
In our current implementation, a subscription with self.topic set to None will not be able to call Subscription.create() (there may be other methods that require a topic to be set on a Subscription but none that I can tell).
Right. I can see the logic there, since you always create a subscription to a topic. But in my mind the right solution is to have a Client().create_subscription(topic=topic_name, subscription_name) or Client().topic(topic_name).create_subscription(subscription_name).
# current logics=Subscription(topic_name, client=Client())
ifnots.exists():
s.create()
The alternative would be:
ifnotClient().subscription_exists(): # create subscriptions=Subscription(subscription_name) # always produces a fully working object
Can you help me understand when a Subscription should have a Topic and when it doesn't matter?
The only use case I can think of is listing subscriptions to a given topic (rather than all subscriptions in the active project). This is, sort of, a convenience method. So the long form is:
Also, can you give an example of a "a subscription resource name to work with and not know the topic"?
If I want to give a third party developer in my project a minimum scope of access, I might give that person a subscription.pull role but not a topic.get permission. This way, the developer may access the data but not modify the access points. (see https://cloud.google.com/pubsub/docs/access_control)
Activity
kir-titievsky commentedon May 4, 2017
@garrettjonesgoogle @lukesneeringer
dhermes commentedon May 4, 2017
@kir-titievsky
FWIW, the behavior you want can already be accomplished (though not with a
Client.subscriptionfactory)RE: "only be given a subscription resource name to work with and not know the topic." Won't the topic be embedded in the subscription resource name?
[-]Create subscriber client without a topic client[/-][+][Pub/Sub] Add a bare Subscriber factory to Client[/+]dhermes commentedon May 4, 2017
@kir-titievsky In our current implementation, a subscription with
self.topicset toNonewill not be able to callSubscription.create()(there may be other methods that require atopicto be set on aSubscriptionbut none that I can tell).Can you help me understand when a
Subscriptionshould have aTopicand when it doesn't matter? Also, can you give an example of a "a subscription resource name to work with and not know the topic"?Update: I see now
Thanks
kir-titievsky commentedon May 4, 2017
@dhermes Thank you for the prompt consideration! Good points. I did not consider the
pubsub.Topic()andpubsub.Subscription()factories since they are not used in the documentation examples. We can fix that bit.That said, this still seems really not super great:
pubsub.Client().topic()but a subscription but a subscription aspubsub.Client().topic().subscription()?pubsub.Subscription()andpubsub.Topic()are the right way to create these, but both of these factories take a client as an argument. So you still have to do:Which would have been at least consistent, but we provide a much more comfortable interface for the topic:
topic = pubsub.Client().topic()which leads me to expect the same for a subscription and also ask why would there be two separate interfaces for creating a subscription (or a topic) one of which is mildly (thepubsub.Topic(name, client=pubsub.Client()) and the other is inconsistent between Topic and Subscriber. The way this was solved in Java is to have separate data and admin classes (Subscriber and SubscriberAdmin). Perhaps a better thing to do is to say that Client() holds all admin operations, like list_topic() and list_subscription() as well as factories for creating subscribers and topics.True. This is not an issue of keeping the topic name from the user. It's about not requiring the user to know the topic name in order to create a subscription in the first place.
Adding subscription factory on Pub / Sub client.
dhermes commentedon May 4, 2017
@kir-titievsky
pubsub.Subscriptionandpubsub.Topicare not factories, they are just the classes (but they ARE part of the public interface).I sent #3370 to add a
Subscriptionfactory, so it should be all good?kir-titievsky commentedon May 4, 2017
Right. I can see the logic there, since you always create a subscription to a topic. But in my mind the right solution is to have a
Client().create_subscription(topic=topic_name, subscription_name)orClient().topic(topic_name).create_subscription(subscription_name).The alternative would be:
or at least
We are getting into matters of taste, I think.
The only use case I can think of is listing subscriptions to a given topic (rather than all subscriptions in the active project). This is, sort of, a convenience method. So the long form is:
The possibly neater version is
There are two distinct API methods.
If I want to give a third party developer in my project a minimum scope of access, I might give that person a subscription.pull role but not a topic.get permission. This way, the developer may access the data but not modify the access points. (see https://cloud.google.com/pubsub/docs/access_control)
kir-titievsky commentedon May 4, 2017
@dhermes Wrote that last comment before seeing yours. Would you please give an example of how the usage changes with your PR?
dhermes commentedon May 4, 2017
@kir-titievsky
The usage doesn't "change" (i.e. the PR is an additive change, so it adds something new). The newly enabled behavior is as you mapped out:
Let's discuss further on the PR?
Adding subscription factory on Pub / Sub client.
dhermes commentedon May 16, 2017
@kir-titievsky We can open a new issue if there are thing's you'd like to discuss?
Some of your comments above indicate that you'd like a way for users to bundle object creation and a call to
reload()with a single helper / method.kir-titievsky commentedon May 16, 2017
dhermes commentedon May 16, 2017
👍
Adding subscription factory on Pub / Sub client.