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.

Sign up for GitHub

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

Jump to bottom

Add json convenience methods to websocket message objects #897

Merged
merged 3 commits into from
Jun 3, 2016
Merged

Add json convenience methods to websocket message objects #897

merged 3 commits into from
Jun 3, 2016

Conversation

sseg
Copy link
Contributor

@sseg sseg commented Jun 2, 2016

Resolves #862

This change adds a json method to the aiohttp.websocket.Message object and a receive_json coroutine method to the aiohttp.web_ws.WebSocketResponse object.

@sseg
Copy link
Contributor Author

sseg commented Jun 2, 2016

@asvetlov is there auto-documentation for the websocket.Message class?

@sseg sseg force-pushed the ws_json branch 2 times, most recently from 45f36e8 to d1438b6 Compare June 2, 2016 19:13
@coveralls
Copy link

coveralls commented Jun 2, 2016 •
edited
Loading

Coverage Status

Coverage decreased (-0.01%) to 98.2% when pulling d55740d on sseg:ws_json into c059e0b on KeepSafe:master.

asvetlov
asvetlov reviewed Jun 2, 2016
View reviewed changes
aiohttp/websocket.py
@@ -69,6 +69,15 @@
MSG_SIZE = 2 ** 14


MessageBase = collections.namedtuple('Message', ['tp', 'data', 'extra'])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use _MessageBase name

@asvetlov
Copy link
Member

asvetlov commented Jun 2, 2016

Yes, it is: http://aiohttp.readthedocs.io/en/stable/api.html#aiohttp.websocket.Message

Message.json() should be documented with versionadded also.

@sseg
Copy link
Contributor Author

sseg commented Jun 3, 2016

Sure, are you targeting version 0.23 for this?

@coveralls
Copy link

coveralls commented Jun 3, 2016 •
edited
Loading

Coverage Status

Coverage decreased (-0.1%) to 98.097% when pulling a56290a on sseg:ws_json into c059e0b on KeepSafe:master.

@asvetlov
Copy link
Member

asvetlov commented Jun 3, 2016

aiohttp 0.22 was not released yet :)

@sseg
Copy link
Contributor Author

sseg commented Jun 3, 2016

👍 bumping back to 0.22

asvetlov
asvetlov reviewed Jun 3, 2016
View reviewed changes
aiohttp/web_ws.py
@@ -285,6 +285,15 @@ def receive_bytes(self):
msg.data))
return msg.data

@asyncio.coroutine
def receive_json(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Signature should be def receive_json(self, *, loads=json.loads)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, good catch.

@asvetlov asvetlov merged commit 8cf1a33 into aio-libs:master Jun 3, 2016
@asvetlov
Copy link
Member

asvetlov commented Jun 3, 2016

Thanks!

@asvetlov asvetlov mentioned this pull request Jun 3, 2016
Closed
@lock
Copy link

lock bot commented Oct 29, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Oct 29, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
outdated
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ClientWebSocketResponse with a json method
3 participants
@sseg @coveralls @asvetlov