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

Raise when calling wait_for_confirms on a closed channel #428

Conversation

@mitio
Copy link
Contributor

mitio commented Jul 18, 2016

I'm proposing this change based on the discussion and feedback I got in #425.

If a channel is already closed it will receive no new frames and waiting
for confirmations will most likely hang and timeout unless the frames
were somehow already received.

If one wishes to wait for confirmations it’s probably the case that the
user doesn’t expect the channel to be closed and we should notify them
about it by failing early. A channel might be closed by RabbitMQ in case
of a channel-level error for example due to a publishing error.

If a channel is already closed it will receive no new frames and waiting
for confirmations will most likely hang and timeout unless the frames
were somehow already received.

If one wishes to wait for confirmations it’s probably the case that the
user doesn’t expect the channel to be closed and we should notify them
about it by failing early. A channel might be closed by RabbitMQ in case
of a channel-level error for example due to a publishing error.
@michaelklishin michaelklishin self-assigned this Jul 18, 2016
@michaelklishin michaelklishin merged commit c0c1b0b into ruby-amqp:master Jul 18, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@michaelklishin
Copy link
Member

michaelklishin commented Jul 18, 2016

Thank you!

@mitio mitio deleted the mitio:wait-for-confirms-raise-on-closed-channel branch Jul 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.