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

Add JSONResponse #599

Merged
merged 2 commits into from
Oct 31, 2015
Merged

Add JSONResponse #599

merged 2 commits into from
Oct 31, 2015

Conversation

sloria
Copy link
Contributor

@sloria sloria commented Oct 28, 2015

closes #592

Let me know if any further changes need to be made.

@@ -810,3 +812,20 @@ def write_eof(self):
if body is not None:
self.write(body)
yield from super().write_eof()


def json_dumps(data):
Copy link
Member

Choose a reason for hiding this comment

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

I suggest

class JSONResponse(Response):

    def __init__(self, data, *, body=None, status=200,
                 reason=None, headers=None, content_type='application/json',
                 dumps=json.dumps):
        super().__init__(text=text, status=status, reason=reason,
                         content_type=content_type)

@asvetlov
Copy link
Member

My former comment was wrong.
See updated proposal:

class JSONResponse(Response):

    def __init__(self, data=sentinel, *, text=None, body=None, status=200,
                 reason=None, headers=None, content_type='application/json',
                 dumps=json.dumps):
        if data is not sentinel and (text of body):
            raise ValueError('only the one of data, text or body should be specified')
        text = dumps(data)
        super().__init__(text=text, body=body, status=status, reason=reason,
                         content_type=content_type)

Also, pass `text` rather than `body`
@sloria
Copy link
Contributor Author

sloria commented Oct 29, 2015

Changes made. Let me know if further changes are necessary or if I should squash the commits.

@sloria
Copy link
Contributor Author

sloria commented Oct 29, 2015

What should happen if none of data, text, or body are passed?

The current behavior is not ideal:

JSONResponse()  # TypeError: <object object at 0x10062e400> is not JSON serializable

I can raise an error:

        if not any([data is not sentinel, text, body]):
            raise ValueError(
                'one of data, text, or body must be specified'
            )

but I'm not sure if we want to allow setting any of those attributes after a JSONResponse is instantiated, in which case I can do the following instead:

        if data is not sentinel:
            text = dumps(data)
        super()...

@asvetlov asvetlov merged commit b7ec83c into aio-libs:master Oct 31, 2015
@asvetlov
Copy link
Member

I've merged but replaced JSONResponse class with json_response function as @popravich suggested.

@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.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add json_response funciton
3 participants