Allow sending multiple messages in bulk

[olzhasar]

This is a draft implementation for #399

It adds support for sending multiple messages to the core layer only. It probably does not make much sense to add that to the pubsub layer, cause the underlying redis publish command only supports a single message.

I had to add an additional argument to the lua script for group sending, I hope that's not a big deal. But let me know if it is, I guess we can make the script dynamic.

[olzhasar]

@carltongibson

Are we sure about changing the signature of the existing send, rather than adding a new method here? Like, we don't use type hints yet, but think through the change there... from single message type to a union of that and an iterable of such... — does that look like a change we'd make?

I initially thought the same way and used two separate methods, but @bigfootjon's comment made me change my mind on that. I agreed cause I thought that typing is out of scope for everything django-related 😁

I'm slightly wary of adjusting the existing method, as we've had regressions in the Lua scripting before

This is a legit concern. I'm actually okay with closing this PR if risks outweigh possible benefits from this feature. For my own use case, I've already made a workaround in place :)

[bigfootjon]

I think the benefits are there so I'd like to see this landed, but if @carltongibson would rather be conservative here I'll defer to his wisdom. @olzhasar would you mind undoing my recommendation and split up each method? Additionally, when you split things up please split the Lua script as well to make sure we don't regress

[carltongibson]

("Wisdom" here means only "been bitten by more innocent looking things enough times to be wary" 😅)

[olzhasar]

I think the benefits are there so I'd like to see this landed, but if @carltongibson would rather be conservative here I'll defer to his wisdom. @olzhasar would you mind undoing my recommendation and split up each method? Additionally, when you split things up please split the Lua script as well to make sure we don't regress

I've updated the PR splitting the methods as discussed, and preserving the old lua script for group_send. Tried to reuse as much code as I could in the group sending methods. It's still not ideal, but going any further would likely lead to a complete redesign, which arguably may not be worth it for this particular feature.

[amirreza8002]

hi is this supposed to be messages? or message?

and this supposed to be args? or *args?

[olzhasar]

hi is this supposed to be messages? or message?

and this supposed to be args? or *args?

Hey,

The first one is a great catch, fixed that.

As for the second thing, it generally can work either way, but I believe the current version leaves more room for future signature changes, e.g. introducing additional positional arguments.

[olzhasar]

@carltongibson any updates on this one?

[carltongibson]

Hi @olzhasar — sorry for the slow follow up here 🤹 Just to let you know, I'm working currently on some issues on asgiref, and an update for Daphne. I'll get back to channels-redis after that, and this PR is on my list. Thanks for your patience. 🎁