-
Notifications
You must be signed in to change notification settings - Fork 197
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
Rewrites RedisChannelLayer.receive specific channel section #165
base: main
Are you sure you want to change the base?
Conversation
* also properly receives specific channels of different prefixes
Hi @augonis. Interesting... So, tests... What’s our new buffer meant to offer that |
The ReceiveBuffer is much more than a My goal was to simplify the control logic with the locks for the receiving as I suspect something there was behind the mystery bug. I'll work on some tests, not familiar with pytest, but I'll learn. |
* adds some tests for ReceiveBuffer
I added tests for the receive task handling, as basic receive is covered by existing tests. P.S. I've put a limit on how many messages per specific channel |
I wondering if we can tie this into Overall this looks nice. It's my summer project to put together a set of decent releases for Daphne, Channels and here, so let's get this in. Thanks for the input! 🏅 |
Doable, but semantically and implementation wise it would muddy the meaning of
|
Yes... but it should be exposed, and does it merit another config option? (Not sure...) |
Should I remove the limit to unblock this PR and open a separate issue? |
Just read django/channels#134, sounds similar my problem, this PR might address it. |
What's the status with this? |
It just needs some time to review properly. I have some issues around Channels' AsyncHttpConsumer at the top of my list, and then channels_redis after that. |
There's also the previously-existing memory leak related to channels that are written to but never written from, in #384 , which I don't advocate fixing in this commit because it's already big enough. |
I'd expect receive_buffers to be long lived and few in number. |
I had a mystery bug where the channel_layer would stop receiving on specific channels after a bunch of receives got canceled.
Unable to reproduce the bug in clean conditions and unable to figure out what exactly was wrong, I just rewrote the suspect (
RedisChannelLayer.receive
).My bug is gone and the tests still pass, so the pull request.
As a bonus, now simultaneously receiving on '!' channels with different prefixes should work (e.g. channel names produced by
redisChannelLayer.new_channel(prefix='something')
), witch was not the case before.