-
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
config: add Dialer option to connect through a proxy #296
base: master
Are you sure you want to change the base?
Conversation
HaoweiCh
commented
May 7, 2020
•
edited
Loading
edited
I want to vote for accept this PR. We are looking for a way to manage NSQ connection, and a custom dialer solved (partially) our problem. It's a convenient thing. |
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.
Left a few comments, thanks!
// Dialer affect connection when dialing an nsqd. Overwrite this to connect over proxy. | ||
// | ||
// Conflict with options LocalAddr and DialTimeout. | ||
Dialer Dialer `opt:"dialer"` |
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.
We need to decide how to handle configuration specified for this opt
, e.g. for command line utilities in nsqd
that use --consumer-opt
.
See: https://github.com/nsqio/nsq/blob/master/apps/nsq_to_file/nsq_to_file.go#L131-L135
And an example of how we handle tls_config
: https://github.com/nsqio/go-nsq/blob/master/config.go#L388-L475
@@ -101,6 +107,10 @@ type Config struct { | |||
// LocalAddr is the local address to use when dialing an nsqd. | |||
// If empty, a local address is automatically chosen. | |||
LocalAddr net.Addr `opt:"local_addr"` | |||
// Dialer affect connection when dialing an nsqd. Overwrite this to connect over proxy. | |||
// | |||
// Conflict with options LocalAddr and DialTimeout. |
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.
Correct, this does conflict. We should probably error when a dialer
and either of those two config fields are set.
@@ -101,6 +107,10 @@ type Config struct { | |||
// LocalAddr is the local address to use when dialing an nsqd. | |||
// If empty, a local address is automatically chosen. | |||
LocalAddr net.Addr `opt:"local_addr"` | |||
// Dialer affect connection when dialing an nsqd. Overwrite this to connect over proxy. |
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.
Let's use:
Dialer will be called to connect to an nsqd when set, e.g. through a proxy