Allow for a redundant AMQP URL #912
Conversation
dviramontes
force-pushed
the
dviramontes/amqp_0_9-retry-dial
branch
from
d158301
to
8ba3c2a
Compare
lib/input/reader/amqp_0_9.go
Outdated
| @@ -37,6 +41,7 @@ type AMQP09BindingConfig struct { | |||
| // AMQP09Config contains configuration for the AMQP09 input type. | |||
| type AMQP09Config struct { | |||
| URL string `json:"url" yaml:"url"` | |||
| URLs string `json:"urls" yaml:"urls"` | |||
There was a problem hiding this comment.
A comma separated list is good for environment variable interpolation, but having an explicit list of urls with a []string here makes the config cleaner, so I've actually been doing both in other places: https://github.com/Jeffail/benthos/blob/master/lib/input/kafka.go#L249, would be good to match that behaviour here.
lib/input/reader/amqp_0_9.go
Outdated
| if err != nil { | ||
| return fmt.Errorf("AMQP 0.9 Connect: %s", err) | ||
| } | ||
| if a.conf.URL != "" { |
There was a problem hiding this comment.
This check needs to be flipped as the old url field has a default value, so when it's not present in a config it'll still have that default value. Ideally we should have a []string of urls that is by default empty, and when one or more urls are present there we use that value, otherwise we go back to url for backwards compatibility.
| @@ -101,20 +103,26 @@ You can access these metadata fields using | |||
|
|
|||
| ## Fields | |||
|
|
|||
| ### `url` | |||
| ### `urls` | |||
There was a problem hiding this comment.
result of running make docs not sure if this step should be part of the PR or maybe its automated ?
dviramontes
changed the title
|
Thanks @dviramontes, looks good. There's a couple of changes I want to make and I'm going to copy this over to the output as well, but I'll merge first. |
addresses #909