asgi_ipc icon indicating copy to clipboard operation
asgi_ipc copied to clipboard

Suggestion: Raise a ValueError exception if receive()'s "channels" argument is not of type "list".

Open danielniccoli opened this issue 8 years ago • 7 comments

Yes, I should have read the documentation more carefully, especially the part where it says

receive(channels, block=False), a callable that takes a list of channel names as unicode strings […]*

It was also tempting to write

channel_layer.send("my_channel", {"text": "Hello"})
channel_layer.receive("my_channel")

and pass a string to the receive method instead of a list, just as in the send() method

Took me a moment to figure out why I wouldn't receive any messages. In asgi_ipc.py#L67 the channels argument is passed to list() and therefore I was listening to a number of channels, but not to what I thought I were listening.

My suggestion is to raise a ValueError exception if the channels argument is not of type "list". Makes debugging a little quicker for those who happen to have overlooked that part in the docs.

* Source: https://channels.readthedocs.io/en/latest/asgi.html

danielniccoli avatar Nov 18 '16 23:11 danielniccoli

Yes, this would be a sensible thing to check for. The base asgiref layer has functions to check channel names already, it's possibly something that could be rolled in there,

andrewgodwin avatar Nov 22 '16 19:11 andrewgodwin

@andrewgodwin should assertion error be raised here like this ?

navyad avatar Dec 12 '16 18:12 navyad

That would work, alternately make this (https://github.com/django/asgiref/blob/master/asgiref/base_layer.py#L104) have a version that validates multiple channels as a list.

andrewgodwin avatar Dec 13 '16 07:12 andrewgodwin

@andrewgodwin list version validation method added https://github.com/django/asgiref/pull/8

navyad avatar Dec 13 '16 11:12 navyad

Thanks, I've merged that in. I'll need to do a new asgiref release before a change can be landed for this as the dependency needs changing.

andrewgodwin avatar Dec 13 '16 17:12 andrewgodwin

@andrewgodwin Cool.

navyad avatar Dec 13 '16 17:12 navyad

Not really - the asgiref implementation of valid_channel_names does not offer the receive argument that we'd need to pass through here. This issue can be resolved via #24 once django/asgiref#18 is merged and released.

rixx avatar Apr 07 '17 21:04 rixx