Skip to content
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

apply custom consumer tag to consumer if supplied #93

Conversation

andreaseger
Copy link
Contributor

@andreaseger andreaseger commented Jun 3, 2016

This fixes #92 and creates the consumer with a custom consumer_tag if one is supplied, otherwise the previous behaviour of using the auto-generated one is maintained.

Some Notes:

  • I placed the consumer_tag as third parameter in Channel#basic_consume to keep the interface the same as the delegated java method.
  • in the recover_from_network_failure case I recreate a consumer with the same consumer tag - so it would use a custom supplied one. I'm not sure in which cases this method is exaclty called and if there are any edge cases to consider when reusing the same consumer_tag.
  • I wasn't entirely sure where to put the specs for this, but the basic_consume_specs seemed like a good place.

@@ -614,10 +614,14 @@ def basic_get(queue, auto_ack)
end
end

def basic_consume(queue, auto_ack, consumer)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking API change but OK, we'll bump the version as it's better than any of the alternatives I can think of.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think is it really a breaking change?

if you call it with 3 parameter the last one will be consumer - this is the same behaviour as before.
if you call it with 4 you have both consumer_tag and consumer

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, in a decade with Ruby I haven't seen a method where arguments with default values weren't at the end of the argument list. That indeed works as you describe:

2.2.2 :001 > def abc(a, b, c = nil, d)
2.2.2 :002?>   [a, b, c, d]
2.2.2 :003?>   end
 => :abc
2.2.2 :004 > abc(1, 2, 3, 4)
 => [1, 2, 3, 4]
2.2.2 :005 > abc(1, 2, 4)
 => [1, 2, nil, 4]

Perfect, I'm going to QA it right away then. Thank you.

@michaelklishin michaelklishin self-assigned this Jun 3, 2016
@andreaseger
Copy link
Contributor Author

sorry for the typo in the specs - somehow missed that

@michaelklishin michaelklishin merged commit 83a532f into ruby-amqp:master Jun 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

supplied consumer_tag not used for consumer
2 participants