search

Polyconseil / aioamqp

AMQP implementation using asyncio
Other
280 stars 88 forks source link

Close cleanup #88

Closed foolswood closed 8 years ago

foolswood commented 8 years ago

An attempt to clean up what's going on around closing (especially in the tests). I believe this covers what Leth's PR was getting at.

I'm seeing some issues in this kind of area, but am struggling to see how it hangs together.

foolswood commented 8 years ago

Any comments on this?

foolswood commented 8 years ago

Is this ok? I am concerned that it is going to go stale.

dzen commented 8 years ago

Hello @foolswood

Thank you for your recent PR. We will look them closely.

foolswood commented 8 years ago

Bump

foolswood commented 8 years ago

I'm running in production with all of the patches I've submitted, no issues seen.

RemiCardona commented 8 years ago

Thanks for the patch, I'll take a look as time allows.

RemiCardona commented 8 years ago

@foolswood I've pushed a bunch of commits to master, some of them directly inspired by the work you did in this PR. We'll probably cut a new release soon as the changes are starting to pile up and I want this code to be battle tested before moving on to other, more invasive, changes.

If you could test the current master and report back, I'd really appreciate it.

Thanks again for your contributions and your patience!

Cheers

foolswood commented 8 years ago

As I suspect you've guessed we have been running on a fork for some months now.

At one point we were seeing bizarre intermittent disconnection issues. A lot of this clean up work was done attempting to get to the bottom of them. They haven't occurred for months now. I don't know exactly what fixed it, maybe it was something in the clean up.

I'm afraid that I need features from all 3 of the PRs to be present before I would be happy to switch back to master. The prospect of going back to a situation where we may have random disconnects without the tools to debug them is not an opportunity I cherish.

RemiCardona commented 8 years ago

AIUI, the last PR from you I need to pull is the "channel cancel" bug, all the other bugs should be fixed. Let me know if anything else is missing.

Thanks again.