Skip to content

Remove UJSONResponse#1047

Merged
JayH5 merged 4 commits intomasterfrom
rm-ujsonresponse
Nov 8, 2020
Merged

Remove UJSONResponse#1047
JayH5 merged 4 commits intomasterfrom
rm-ujsonresponse

Conversation

@JayH5
Copy link
Contributor

@JayH5 JayH5 commented Sep 6, 2020

Closes #901

It's easy enough to implement your own UJSONResponse response class if you need it:

from typing import Any

import ujson
from starlette.responses import JSONResponse

class UJSONResponse(JSONResponse):
    def render(self, content: Any) -> bytes:
        return ujson.dumps(content, ensure_ascii=False).encode("utf-8")

@euri10
Copy link
Contributor

euri10 commented Sep 6, 2020

I'm all in for that removal !
Not a blocker on my side but could we add the example you provided (or even better with orjson ) in docs/responses.md ?

@JayH5 JayH5 added the clean up Refinement to existing functionality label Sep 11, 2020
@JayH5
Copy link
Contributor Author

JayH5 commented Sep 12, 2020

@euri10 I wrote a bit about custom JSON serialization in responses.md. lmk what you think!

@euri10
Copy link
Contributor

euri10 commented Sep 12, 2020

@euri10 I wrote a bit about custom JSON serialization in responses.md. lmk what you think!

looks perfect ! 👍

@lovelydinosaur
Copy link
Contributor

Fab yup!
Will need calling out in release notes and a median version bump.
Great stuff, thank you!

@erewok
Copy link
Contributor

erewok commented Nov 8, 2020

@JayH5, this would have been a good one to include in the 0.14.0 release. Can we merge it and get it released after the fact as 0.14.1? It's pretty close to the "breaking-change" semver bump and with the recent release of 0.14.0 most users have probably not upgraded yet?

@JayH5
Copy link
Contributor Author

JayH5 commented Nov 8, 2020

@erewok yeah sorry, I didn't want it to get in the way of all the fixes in "0.13.9" and then when we switched to 0.14 I forgot about it.

Happy to squeeze it in if you want to do that? I won't be able to organise a release right now (haven't actually done one before), but if you want to merge this and do one be my guest.

@erewok
Copy link
Contributor

erewok commented Nov 8, 2020

@JayH5, no worries!

I do think we could take advantage of the "breaking change" version bump to merge this (and probably any other test or documentation changes that are easily to include) and make another release.

Would you be up for merging this and issuing a release? (For reference: https://www.python-httpx.org/contributing/#releasing)

@JayH5
Copy link
Contributor Author

JayH5 commented Nov 8, 2020

(and probably any other test or documentation changes that are easily to include)

I don't see any other obvious candidate PRs. Will give this a go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clean up Refinement to existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider removing UJSONResponse from core

4 participants