Musings towards SubscribeNotification for Connection #42
Conversation
Signed-off-by: Ed Warnicke <hagbard@gmail.com>
|
Note: This PR is mostly meant to be a conversation starter on how we get to being able to subscribe to connections without needing things like: #36 |
| // bufferSize is used to set the buffer size of the go channel returned. If the channel's | ||
| // buffer is full, the notifications will not be delivered into it. When the provided ctx is 'Done' | ||
| // the return go channel will be closed and no further notifications will be deli | ||
| SubscribeNotification(ctx context.Context, event Message, bufferSize int) (chan Message, error) |
There was a problem hiding this comment.
Some comments on design decisions here.
I started with just moving the
SubscribeNotification(notifChan chan Message, event Message) (SubscriptionCtx, error)from Channel up to Connection. As I was implementing I realized a few places for improvment:
First, everything at Connection Level uses a context.Context, so I added that:
SubscribeNotification(ctx context.Context, notifChan chan Message, event Message) (SubscriptionCtx, error)Which led me to wanting to clean up the notifChan when the ctx.Done() was closed. Normal Go lang idioms around channels are that the writer owns them and is responsible for closing them. So normally since the Connection is writing to them, it would create and return the channel.
Looking at the code it was clear the reason for the channel being passed in was to control the bufferSize. I can absolutely see why controlling the bufferSize from outside is important.
So I evolved it to:
SubscribeNotification(ctx context.Context, event Message, bufferSize int) (chan Message, error)
|
Superseded by #80 |
Signed-off-by: Ed Warnicke hagbard@gmail.com