nsqio / go-nsq

The official Go package for NSQ
MIT License
2.59k stars 444 forks source link

Consumer should receive connection to NSQ as an argument to constructor #220

Closed corpix closed 6 years ago

corpix commented 6 years ago

Hello. Why do you have Consumer connecting to NSQ?

Let me provide some examples. Current implementation forces me to use consumer in this manner:

import (
    nsq "github.com/nsqio/go-nsq"
)

consumer, err := nsq.NewConsumer(...)
...

consumer.ConnectToNSQD(...)

But I think that Consumer should not know ho to connect to NSQ. It's just a dumb consumer, it should have single responsibility - consume data! From my point of view using a Consumer should look like this:

import (
    "io"
    nsq "github.com/nsqio/go-nsq"
)

var (
    conn io.ReadWriteCloser
    stream <-chan []byte
    err error
)

conn, err = nsq.Connect(...)
...
defer conn.Close()

consumer := nsq.NewConsumer(conn, topic, channel, ...)
stream = consumer.Consume()

for message := range stream {
    ...
}

There are many other functions in the package which cause state transitions, like SetLogger. It should be passed to constructor as argument or as part of some configuration structure.

I think this would make client library more easy to use, more testable, etc.

What do you think about this?

ploxiln commented 6 years ago

Consumer can connect to multiple nsqd, and it can re-connect after disconnection from an nsqd. Consumer can be instructed to connect to nsqlookupd instead, in which case it looks up the topic in nsqlookupd, and then connects to all the returned nsqd, and the re-connect logic is different. The topic and channel are sort of part of the protocol connection handshake.

The thing which does not know how to connect to nsqd is the handler function - that's the simple dumb consumer. The Consumer is the sophisticated connection handler. And multiple instances of the handler function can be run concurrently if max_in_flight > 1.

So, there are some good reasons why the current API design is the way it is, and it's unlikely we would change it because that would break compatibility with lots of current users. If you feel strongly, however, you could try to write your own competing "simpler" nsq client library for go, and some people might try it.

corpix commented 6 years ago

Ah, okay, I see things strongly relates on each other. Probably I'll try to create my own package if I have much time. Love NSQ, it is so simple to use <3 Thank you for the answer!