-
Notifications
You must be signed in to change notification settings - Fork 442
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
consumer: ability to disconnect from nsqd #91
Conversation
@@ -477,8 +483,6 @@ func (r *Consumer) ConnectToNSQD(addr string) error { | |||
conn.Close() | |||
} | |||
|
|||
r.pendingConnections[addr] = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could have separated these commits better, but this was racey and needed to be moved into the critical section inside the lock.
I'm going to add mirroring functionality to remove |
RFR @jehiah |
|
||
idx := indexOf(addr, r.nsqdTCPAddrs) | ||
if idx == -1 { | ||
return errors.New(fmt.Sprintf("unknown nsqd TCP address %s", addr)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mixed feelings as to weather or not this should error since we don't expose the list of where you are connected to. We don't typically expose named errors, but perhaps this would be a fair spot for return NotConnectedError
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NotConnectedError
seems good to me
updated |
same for |
looks good; squash beyond that |
Yes, they're consistent now |
cd8e94f
to
ed86be0
Compare
🔨 |
consumer: ability to disconnect from nsqd
this adds a function that allows the end-user to remove
addresses from the list of
nsqd
for situations wherethey're not using
nsqlookupd
and instead maintainingtheir own list.