-
Notifications
You must be signed in to change notification settings - Fork 227
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
#1039 Migrate azure/go-amqp to version 1.0.5 #1040
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,42 +12,76 @@ import ( | |
// Option is the function signature required to be considered an amqp.Option. | ||
type Option func(*Protocol) error | ||
|
||
// WithConnOpt sets a connection option for amqp | ||
func WithConnOpt(opt amqp.ConnOption) Option { | ||
type SendOption func(sender *sender) | ||
type ReceiveOption func(receiver *receiver) | ||
|
||
// WithConnOpts sets a connection option for amqp | ||
func WithConnOpts(opt *amqp.ConnOptions) Option { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since you're using pointer semantics in those options, be careful with races - users might not be aware that you're passing the pointer around (instead of copy) and this could lead to surprises for users. I suggest using value semantic to avoid races and these surprises. |
||
return func(t *Protocol) error { | ||
t.connOpts = append(t.connOpts, opt) | ||
t.connOpts = opt | ||
return nil | ||
} | ||
} | ||
|
||
// WithConnSASLPlain sets SASLPlain connection option for amqp | ||
func WithConnSASLPlain(username, password string) Option { | ||
return WithConnOpt(amqp.ConnSASLPlain(username, password)) | ||
func WithConnSASLPlain(opt *amqp.ConnOptions, username, password string) Option { | ||
opt.SASLType = amqp.SASLTypePlain(username, password) | ||
return WithConnOpts(opt) | ||
} | ||
|
||
// WithSessionOpt sets a session option for amqp | ||
func WithSessionOpt(opt amqp.SessionOption) Option { | ||
// WithSessionOpts sets a session option for amqp | ||
func WithSessionOpts(opt *amqp.SessionOptions) Option { | ||
return func(t *Protocol) error { | ||
t.sessionOpts = append(t.sessionOpts, opt) | ||
t.sessionOpts = opt | ||
return nil | ||
} | ||
} | ||
|
||
// WithSenderLinkOption sets a link option for amqp | ||
func WithSenderLinkOption(opt amqp.LinkOption) Option { | ||
// WithSenderOpts sets a link option for amqp | ||
func WithSenderOpts(opt *amqp.SenderOptions) Option { | ||
return func(t *Protocol) error { | ||
t.senderLinkOpts = append(t.senderLinkOpts, opt) | ||
t.senderOpts = opt | ||
return nil | ||
} | ||
} | ||
|
||
// WithReceiverLinkOption sets a link option for amqp | ||
func WithReceiverLinkOption(opt amqp.LinkOption) Option { | ||
// WithReceiverOpts sets a link option for amqp | ||
func WithReceiverOpts(opt *amqp.ReceiverOptions) Option { | ||
return func(t *Protocol) error { | ||
t.receiverLinkOpts = append(t.receiverLinkOpts, opt) | ||
t.receiverOpts = opt | ||
return nil | ||
} | ||
} | ||
|
||
// WithReceiveOpts sets a receive option for amqp | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you please explain why we need WithReceiverOpts and WithReceiveOpts (also for send)? Is there a way to only have one? These changes complicate the API and I'm having a hard time understanding why we need those very similar options (and I guess new CE users would also be slightly confused?) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IIUC, this is for the sender/receiver construction, which currently does not support options. Do we need this new functionality at the cost of a more complex user API? |
||
func WithReceiveOpts(opt *amqp.ReceiveOptions) Option { | ||
return func(t *Protocol) error { | ||
t.receiveOpts = opt | ||
return nil | ||
} | ||
} | ||
|
||
// WithSendOpts sets a send option for amqp | ||
func WithSendOpts(opt *amqp.SendOptions) Option { | ||
return func(t *Protocol) error { | ||
t.sendOpts = opt | ||
return nil | ||
} | ||
} | ||
|
||
// WithSendOptions sets send options for amqp | ||
func WithSendOptions(opts *amqp.SendOptions) SendOption { | ||
return func(t *sender) { | ||
t.options = opts | ||
} | ||
} | ||
|
||
// WithReceiveOptions sets receive options for amqp | ||
func WithReceiveOptions(opts *amqp.ReceiveOptions) ReceiveOption { | ||
return func(t *receiver) { | ||
t.options = opts | ||
} | ||
} | ||
|
||
// SenderOptionFunc is the type of amqp.Sender options | ||
type SenderOptionFunc func(sender *sender) |
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.
nit: both need doc strings