By default, the virtual host configured in the uri is used #238
Conversation
| "log" | ||
| "os" | ||
| "os/signal" | ||
| "syscall" | ||
| "time" | ||
|
|
||
| amqp "github.com/rabbitmq/amqp091-go" |
There was a problem hiding this comment.
Was this change due to your preference, or to a tool auto-formatting the code?
There was a problem hiding this comment.
Is there other example code that should be formatted as part of this PR?
There was a problem hiding this comment.
go fmt ./... doesn't make this change for me 😕 Are you running go fmt with any special flag?
There was a problem hiding this comment.
go fmt ./...doesn't make this change for me 😕 Are you runninggo fmtwith any special flag?
Yes, you are right, I think my submitted code was automatically formatted by the development tool (goland); I hope you ignore the submission about here. Thanks
| @@ -58,7 +59,6 @@ func setupCloseHandler(exitCh chan struct{}) { | |||
|
|
|||
| func publish(ctx context.Context, publishOkCh <-chan struct{}, confirmsCh chan<- *amqp.DeferredConfirmation, confirmsDoneCh <-chan struct{}, exitCh chan struct{}) { | |||
| config := amqp.Config{ | |||
| Vhost: "/", | |||
There was a problem hiding this comment.
Does other example code use Vhost: when it's not necessary?
There was a problem hiding this comment.
My suggestion is that the configured uri vhost should be used first. In other cases, you can configure it yourself.
Of course it's just my suggestion.
There was a problem hiding this comment.
I'm not sure I understand the benefit of this. IMO, having Vhost field set in the config is very clear, and it's better than relying in the implicit behaviour of URI parsing.
There was a problem hiding this comment.
I'm not sure I understand the benefit of this. IMO, having
Vhostfield set in the config is very clear, and it's better than relying in the implicit behaviour of URI parsing.
The reason why I modified it like this is because after I set the Vhost in the uri, I ran the code and reported an error. After comparison, I found that this was not the vhost I set.
There was a problem hiding this comment.
Maybe this is just my personal understanding. If you think the original way is better, I think my submission can be canceled. At the same time, I am also happy to communicate with you, thank you.
|
I'm not sure I understand the improvement suggested in this PR. IMO, the current state is clear to what vhost the example is connecting to. Removing an explicit option and relying on implicit behaviour is arguably an improvement for an example. |
This is what my PR meant, remove the explicit options and rely on the implicit behavior. |
Use virtual host in uri