aiohttp icon indicating copy to clipboard operation
aiohttp copied to clipboard

Added support orjson for dumps of response

Open KorsaR-ZN opened this issue 4 years ago • 4 comments

What do these changes do?

Added support for JSONEncoder implementations that return bytes instead of string, for example, such as orjson

Are there changes in behavior for the user?

No

Related issue number

No

Checklist

  • [x] I think the code is well written
  • [x] Unit tests for the changes exist
  • [x] Documentation reflects the changes
  • [x] If you provide code modification, please add yourself to CONTRIBUTORS.txt
    • The format is <Name> <Surname>.
    • Please keep alphabetical order, the file is sorted by names.
  • [x] Add a new news fragment into the CHANGES folder
    • name it <issue_id>.<type> for example (588.bugfix)
    • if you don't have an issue_id change it to the pr id after creating the pr
    • ensure type is one of the following:
      • .feature: Signifying a new feature.
      • .bugfix: Signifying a bug fix.
      • .doc: Signifying a documentation improvement.
      • .removal: Signifying a deprecation or removal of public API.
      • .misc: A ticket has been closed, but it is not of interest to users.
    • Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files."

KorsaR-ZN avatar Mar 31 '21 21:03 KorsaR-ZN

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 97.21%. Comparing base (47b8d63) to head (32d5d3d). Report is 3589 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5578   +/-   ##
=======================================
  Coverage   97.21%   97.21%           
=======================================
  Files          41       41           
  Lines        8863     8864    +1     
  Branches     1425     1426    +1     
=======================================
+ Hits         8616     8617    +1     
  Misses        130      130           
  Partials      117      117           
Flag Coverage Δ
unit 97.10% <100.00%> (+<0.01%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Mar 31 '21 21:03 codecov[bot]

I'm not sure this is a good idea. What stops you from making the encoder compliant? Are there any other non-compliant implementations or is it just this one?

I have not seen any other similar implementations.

In fact, the changes is required only in this line, so as not to do a double conversion, I originally did just that, see previous commit.

All other changes can be undone and a compatible encoder can be used there.

KorsaR-ZN avatar Apr 01 '21 15:04 KorsaR-ZN

Sorry, no. You support orjson encoder only without decoding. It looks weird.

In fact, you can write a simple function that accepts JSON dict and returns orjson encoded bytes for passing as data argument to client.get() and family.

There is no need for special support on the aiohttp side.

asvetlov avatar Oct 30 '21 11:10 asvetlov

How to set the content-type header @asvetlov ? by also passing the headers dict?

I think a custom data payload is better

import orjson
import aiohttp
from aiohttp.payload import BytesPayload

class ORJSONData(BytesPayload):
    def __init__(self, value) -> None:
        super().__init__(
            orjson.dumps(value),
            content_type="application/json",
            encoding="utf-8"
        )

async with aiohttp.ClientSession() as session:
    async with session.post(
        "http://json.example.com/",
        data=ORJSONData({
            "foo": "bar"
        })
     ) as response:
        jsonbody = await response.json(loads=orjson.loads)
 

This reduces usability though, that's why builtin encoder/decoder support for orjson would be helpful.

HMaker avatar Nov 16 '22 16:11 HMaker