Don't close subscriber if channel is recoverable.

ScalaConsultants / reactive-rabbit

Reactive Streams driver for AMQP protocol. Powered by RabbitMQ library.

Apache License 2.0

184 stars 40 forks source link

Don't close subscriber if channel is recoverable. #49

Open pribor opened 8 years ago

pribor commented 8 years ago

I made some changes to make the auto recovery work.

Ignore a channel shutdown event if it is a recoverable channel that wasn't shut down by an application.
Try to close the channel in handleCancel method.
QueuePublisher registers shutdown listeners to QueueSubscription.
Simplify QueueSubscription termination.

mkiedys commented 8 years ago

LGTM

dskrvk commented 8 years ago

What's the status here?

mkiedys commented 8 years ago

Hi @pribor! Can you rebase?

pribor commented 8 years ago

Done

DGolubets commented 7 years ago

I tried it: killed rabbit, started it again, but my streams didn't resume..

kerzok commented 7 years ago

Is there any chance that this PR will be accepted? I tried to run it and seems to streams resuming works correctly.

marius-carp commented 7 years ago

It's been a while since this PR is opened, what is the reason for not merging it?

duketon commented 7 years ago

Bump, with the above question.

mkiedys commented 7 years ago

Can someone confirm if this works?

marius-carp commented 7 years ago

I'm using it in staging environment and it seems to be working.

djamelz commented 7 years ago

Hi guys, it works very well ! Is it possible to merge it ? (BTW more than the branch conflict, I was forced to refactor a little bit the code to build it in scala 2.12)

muub commented 7 years ago

Hey folks, We are actively using this fix in production. We had to pin to it in order to continue using this library. We would very much appreciate having this PR merged. If we can help in any way please let me know.

guizmaii commented 7 years ago

What's the pb with this PR ? @mkiedys ? What should we do to make you accept it ?

yspotts commented 7 years ago

Just want to add our voice to the mix. We are having a issue in production caused by this bug. We are going to deploy our own version based on this PR. Would love to see this PR get merged in the near future! Thanks for a great library!

mkiedys commented 7 years ago

The reason why this stayed open so long are:

conflicting reviews suggesting this isn't working
no tests
no clear story how to replicate and verify if it works

I'm looking into this right now.

dskrvk commented 6 years ago

What's the relationship to https://github.com/ScalaConsultants/reactive-rabbit/pull/64?