Skip to content
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

Attempted implementation of AsgiRequest._stream #67

Closed
wants to merge 3 commits into from

Conversation

DanLipsitt
Copy link
Contributor

This is the beginning of an attempt to fix #32. It isn't working. This PR is just for the sake of discussion.

The test I added surfaces an issue I mentioned in #66. AsgiRequest uses dict-style access on Message, but Message does not support it. See below. This could be because my test is wrong.

$ python runtests.py 
Creating test database for alias 'default'...
EEE
======================================================================
ERROR: test_stream_is_readable (channels.tests.test_handler.TestAsgiRequest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/dan/src/channels/channels/tests/test_handler.py", line 14, in test_stream_is_readable
    request = AsgiRequest(message)
  File "/Users/dan/src/channels/channels/handler.py", line 25, in __init__
    self.reply_channel = self.message['reply_channel']
TypeError: 'Message' object is not subscriptable

@andrewgodwin
Copy link
Member

Ah, yes, message in AsgiRequest is meant to just be the raw underlying message dict from ASGI - see here: https://github.com/andrewgodwin/channels/blob/master/channels/handler.py#L208 where it passes Message.content into the view system, which is the raw dict.

@DanLipsitt
Copy link
Contributor Author

OK, maybe working now.

@DanLipsitt
Copy link
Contributor Author

Running up against missing BaseChannelBackend now, so I think that's all I can do for the moment.

@andrewgodwin
Copy link
Member

Ah, that shouldn't be needed anymore. Might be old broken test code - this is why I don't want to write more tests until the new architecture is stable :)

@DanLipsitt
Copy link
Contributor Author

I don't think it has anything to do with the test code. Filed as #69.

@andrewgodwin
Copy link
Member

I implemented _stream basically this exact way, so closing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HttpRequest._stream
2 participants