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

doc: binder example for msgspec #106

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

doc: binder example for msgspec #106

wants to merge 5 commits into from

Conversation

adriangb
Copy link
Owner

@adriangb adriangb commented Sep 12, 2022

cc @jcrist

I'm not covering a bunch of edge cases and additional features (empty bodies, include_in_schema=False, descriptions, etc.) nor am I covering doing the same thing for query/path/etc. params. I think it would be interesting as a 3rd party package though!

@adriangb
Copy link
Owner Author

TODO: write the docs

@codecov-commenter
Copy link

codecov-commenter commented Sep 12, 2022

Codecov Report

Merging #106 (333d195) into main (a292301) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #106   +/-   ##
=======================================
  Coverage   98.44%   98.44%           
=======================================
  Files         127      128    +1     
  Lines        4231     4237    +6     
  Branches      598      598           
=======================================
+ Hits         4165     4171    +6     
  Misses         32       32           
  Partials       34       34           
Impacted Files Coverage Δ
tests/test_docs/advanced/binders/test_msgspec.py 100.00% <100.00%> (ø)

Copy link

@jcrist jcrist left a comment

Choose a reason for hiding this comment

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

First off - this is super cool to see. I like how pluggable all of this is in xpresso! Seems like a well designed API. I'm also impressed by how quickly you wrote this up - not even an hour after I merged the feature into msgspec!

In the medium- to long-term do you see msgspec support living in the docs like this? Or would you pull the functionality defined here into xpresso itself (or a plugin package)? I personally would like to see feature parity between msgspec and pydantic in libraries like this, but I'm definitely biased here :).

bar: int


async def echo_item(items: FromJson[List[Item]]) -> int:
Copy link

Choose a reason for hiding this comment

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

I can see how the backend used for parsing an input type/generating JSON Schema is dispatchable based on a FromJson annotation - can the same be done for the return type? E.g. how would you return a msgspec.Struct type from a handler and have everything still work?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Great question!

We're unfortunately a bit tied to Pydantic for responses, that part isn't as pluggable as I'd like. I'll have to think a bit more about how we could make that more pluggable, it should be possible. I think we may want to have some sort of -> JsonResponse[MyMsgSpecStruct] where JsonResponse somehow tells Xpresso both how to serialize the response and how to build the OpenAPI schema.

Copy link
Owner Author

Choose a reason for hiding this comment

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

@jcrist is msgspec much faster at encoding something like list[Struct] into bytes than it would be to convert it to a list[dict] and then run that through a fast json serializer (like orjson)? I think the list[Struct] -> list[dict] part doesn't currently exist, so I guess this is a theoretical question.

@adriangb
Copy link
Owner Author

In the medium- to long-term do you see msgspec support living in the docs like this? Or would you pull the functionality defined here into xpresso itself (or a plugin package)?

For now I think it will just live in the docs. To be honest, I only have bandwidth to support one version of Xpresso. Even it is very pluggable, there is a non-trivial amount of work required to support something like xpresso-msgspec, especially if there was support for using msgspec to parse complex query params, get good integration into the returning msgspec.Structs directly like you mention above, etc. I do think it would be really interesting to explore at least short term, I just am afraid to commit to maintaining something like that long term. Do you think it would be an interesting experiment to try and build something more in-depth and see how it goes without committing to long-term maintenance?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants