-
Notifications
You must be signed in to change notification settings - Fork 137
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
Configure consumer tag names/prefixes #265
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 |
---|---|---|
|
@@ -45,9 +45,27 @@ | |
end | ||
|
||
it 'sets up a subscription' do | ||
expect(queue).to receive(:subscribe).with(manual_ack: true) | ||
expect(queue).to receive(:subscribe).with(consumer_tag: %r(^hutch\-.{36}$), manual_ack: true) | ||
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. Do I need to double check AMQP/RabbitMQ spec for the maximum 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. That would be a cool touch. (Then we could figure out ways to validate configuration options. Example addition to the Config options' DSL: 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. It "depends", it seems: https://www.rabbitmq.com/amqp-0-9-1-reference.html#basic.consume.consumer-tag it is a 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. Seems like it's 255 bytes max: https://github.com/rabbitmq/rabbitmq-common/blob/rabbitmq_v3_6_0/codegen.py#L178-L179 ... I hope that's the same in 3.4.x as well :) 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. So, now we know that the longest prefix that can be used is: 256 - 1 - 36 = 219 bytes. 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. Ideally Hutch should loudly complain about prefixes that are longer, e.g. log an error and trim the prefix or fail to start. 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. Opted for an "on-create" validation as validating the actual prefix length is weird (why 219 bytes?) and coupled to the implementation. 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. Is the on-create validation sufficient/ok? |
||
worker.setup_queue(consumer) | ||
end | ||
|
||
context 'with a configured consumer tag prefix' do | ||
before { Hutch::Config.set(:consumer_tag_prefix, 'appname') } | ||
|
||
it 'sets up a subscription with the configured tag prefix' do | ||
expect(queue).to receive(:subscribe).with(consumer_tag: %r(^appname\-.{36}$), manual_ack: true) | ||
worker.setup_queue(consumer) | ||
end | ||
end | ||
|
||
context 'with a configured consumer tag prefix that is too long' do | ||
let(:maximum_size) { 255 - SecureRandom.uuid.size - 1 } | ||
before { Hutch::Config.set(:consumer_tag_prefix, 'a'.*(maximum_size + 1)) } | ||
|
||
it 'raises an error' do | ||
expect { worker.setup_queue(consumer) }.to raise_error(/Tag must be 255 bytes long at most/) | ||
end | ||
end | ||
end | ||
|
||
describe '#handle_message' do | ||
|
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.
Thank you!
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.
(This is you coming back to improve the README: 🎩 !)