search

jhnnsrs / rath

rath is an apollo-like graphql client that supports links, multipart uploads, websocket subscriptions with sync and async interface
https://jhnnsrs.github.io/rath/
MIT License
9 stars 3 forks source link

Adds extended JSONEncoder that can serialize pydantic models. #8

Open strue36 opened 2 years ago

strue36 commented 2 years ago

Fixes the issue described in #7

codecov[bot] commented 2 years ago

Codecov Report

Merging #8 (2913c0c) into master (2662ad7) will decrease coverage by 3.31%. The diff coverage is 100.00%.

:exclamation: Current head 2913c0c differs from pull request most recent head b32e08f. Consider uploading reports for the commit b32e08f to get more accurate results

@@            Coverage Diff             @@
##           master       #8      +/-   ##
==========================================
- Coverage   89.68%   86.37%   -3.32%     
==========================================
  Files          30       30              
  Lines         931      925       -6     
==========================================
- Hits          835      799      -36     
- Misses         96      126      +30
Impacted Files Coverage Δ
rath/json/encoders.py 100.00% <100.00%> (ø)
rath/links/aiohttp.py 62.29% <100.00%> (-6.94%) :arrow_down:
tests/test_json.py 100.00% <100.00%> (ø)
rath/links/validate.py 59.15% <0.00%> (-37.28%) :arrow_down:
rath/links/parsing.py 55.00% <0.00%> (-33.89%) :arrow_down:
tests/test_validation.py 73.33% <0.00%> (-23.22%) :arrow_down:
rath/links/split.py 87.87% <0.00%> (-12.13%) :arrow_down:
rath/links/base.py 71.64% <0.00%> (-10.18%) :arrow_down:
rath/links/dictinglink.py 75.00% <0.00%> (-7.15%) :arrow_down:
... and 23 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 2662ad7...b32e08f. Read the comment docs.

jhnnsrs commented 2 years ago

Thanks Sean! I had a similar implementation of this in a dedicated link (Dicting Link), which stirs up the question of what should go into links and what would be considered common functionality to be implemented directly in a terminating link. Do you have any suggestions in what would be considered a spec of a terminating link? I was thinking that maybe the terminating link should only assert that everything is json serializable and raise an error if not, leaving the complex json encoding to another link? Whats your idea?

strue36 commented 2 years ago

I think this also works as a dedicated link. I'm not sure whether it should be a DictingLink as we probably want this link to handle other conversions from complex types to objects that are json serialisable (Such as datetime -> str). It looks like ParsingLink covers this though.

One thing that a custom JSONEncoder does bring is that it handles recursion nicely. I think that the current implementation of DictingLink will only handle top level BaseModels, but will skip over nested types such as List[BaseModel]. (It looks like it also throws away any variables that aren't a BaseModel)

If this conversion is intended to be handled by a ParsingLink this should probably be included in the rath/turms documentation here as it is likely that people will have pydantic models in their turms generated code. It should also be documented that the TerminatingLink expects all variables to be in a specific format. I think your suggestion of validating

Do you think the AIOHttpLink should expose a way to modify the default values of the ClientSession? Even if the conversion of variables is handled in a ContinuationLink, people may want to use a different json serializer such as ujson.