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 support for cbor #619

Merged
merged 9 commits into from
Sep 2, 2021
Merged

Add support for cbor #619

merged 9 commits into from
Sep 2, 2021

Conversation

Erik1000
Copy link
Contributor

@Erik1000 Erik1000 commented Sep 2, 2021

A while back we discussed the ability to use cbor as encoding. Back then, we got that to work and used it since on our fork. I thought that we could make a PR too.

Note

We use it to build a graphql server and haven't encountered any errors yet. Multipart (for the file upload extension) probably won't work just yet, because it still uses serde_json.

@Erik1000
Copy link
Contributor Author

Erik1000 commented Sep 2, 2021

Related issue: #569

@sunli829
Copy link
Collaborator

sunli829 commented Sep 2, 2021

Thank you very much, it looks great! 🙂

@Erik1000
Copy link
Contributor Author

Erik1000 commented Sep 2, 2021

forgot to format that i guess

@sunli829
Copy link
Collaborator

sunli829 commented Sep 2, 2021

forgot to format that i guess

I will solve it, and I will add a feature called cbor, what do you think?

@Erik1000
Copy link
Contributor Author

Erik1000 commented Sep 2, 2021

I will solve it, and I will add a feature called cbor, what do you think?

seems like a good idea 👍

please note that clippy probably will still fail, because this fork was created before c4cfa24
@Erik1000
Copy link
Contributor Author

Erik1000 commented Sep 2, 2021

Should I merge your master into our fork so clippy passes(see latest commit comment)?

@Erik1000
Copy link
Contributor Author

Erik1000 commented Sep 2, 2021

Also I'm currently looking into the multipart code and there's a problem with multer:

I want to reduce code duplication so here I want to reuse the receive_batch_body method.

The problem is that this (and actually all these) methods take body as impl AsyncRead but multer Field.bytes returns bytes::Bytes. Do you have an idea how to overcome this or if not, is it okay if I change the method's signature?

@sunli829 sunli829 merged commit a8d6163 into async-graphql:master Sep 2, 2021
@sunli829
Copy link
Collaborator

sunli829 commented Sep 2, 2021

@sunli829
Copy link
Collaborator

sunli829 commented Sep 2, 2021

I have merged into the master.

@sunli829
Copy link
Collaborator

sunli829 commented Sep 2, 2021

Also I'm currently looking into the multipart code and there's a problem with multer:
I want to reduce code duplication so here I want to reuse the receive_batch_body method.

You can create a new PR to continue to improve. 🙂

@Erik1000
Copy link
Contributor Author

Erik1000 commented Sep 2, 2021

I'll do 👍

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

4 participants