sdk-go icon indicating copy to clipboard operation
sdk-go copied to clipboard

Pubsub topic handling should be pulled out of Send path

Open ericlem opened this issue 5 years ago • 5 comments

Currently the pubsub protocol defines the Sender, Receiver on a general pubsub connection regardless of topic, with the topic specified in the ctx. We should look at splitting Sender and Receiver into separate instances per Topic and Subscription in the v2 API.

With the current model, Send() and Receive() must internally handle connection state in a moderately complex way (within the Send() checking the topic/subscription exists, making sure there is a single internal pubsub Topic or Subscription to handle all calls to each Topic, etc). A lot of this would be made easier if Sender and Receiver were actually independent objects per-topic, allowing this to either happen via passing in an already created pubsub Topic object, or by having an explicit constructor that did this work.

ericlem avatar Apr 28 '20 16:04 ericlem

This is related to the bug seen in https://github.com/cloudevents/sdk-go/issues/472 and would restructure it in a way to remove the underlying complexity that helped cause that bug.

ericlem avatar Apr 28 '20 16:04 ericlem

@slinkydeveloper @yolocs I'll play around with explicit interface changes and see what seems most natural/useful.

ericlem avatar Apr 28 '20 16:04 ericlem

@ericlem it was discussed in past (if i find the thread i'll tell you :smile: ) that the Sender/Receiver interfaces are "per topic/channel/subscription/whatever", So i don't see the problem tbh.

slinkydeveloper avatar Apr 28 '20 16:04 slinkydeveloper

We should look at splitting Sender and Receiver into separate instances per Topic and Subscription in the v2 API.

yeah this will help

The reason the current code is so complex was to support a single client to handle multi-topics and subscriptions.

n3wscott avatar Apr 28 '20 16:04 n3wscott

@ericlem it was discussed in past (if i find the thread i'll tell you 😄 ) that the Sender/Receiver interfaces are "per topic/channel/subscription/whatever", So i don't see the problem tbh.

I don't see it being hard to change over either. I just want to play a little with a few interfaces to make sure I get something that feels natural and consistent with the other protocols in the library (ie, do I pass the Topic into the constructor, or do I pass the client and the topic name into the constructor and let it open the Topic). I'm happy for any input from people (ie, mainly you and scott ;) ) who have been playing with multiple protocols here. It'll probably be a couple days before I get a chance to play with it, but I don't expect it to be that much work in the end.

ericlem avatar Apr 28 '20 16:04 ericlem