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

Reuse the code of Extract when implementing Service. #147

Closed
sunli829 opened this issue Aug 7, 2021 · 5 comments · Fixed by #194
Closed

Reuse the code of Extract when implementing Service. #147

sunli829 opened this issue Aug 7, 2021 · 5 comments · Fixed by #194
Labels
C-enhancement Category: A PR with an enhancement E-easy Call for participation: Experience needed to fix: Easy / not much E-help-wanted Call for participation: Help is requested to fix this issue.
Milestone

Comments

@sunli829
Copy link
Contributor

sunli829 commented Aug 7, 2021

Feature Request

Motivation

I want to implement a Service for GraphQL query execution so as to avoid boilerplate code like the following.

https://github.com/async-graphql/examples/blob/7e3304e406adaff8d28a025becd91867ecae199a/axum/starwars/src/main.rs#L8

I have implemented a GraphQL request extractor, I want to implement Service can reuse it.

But I cannot use RequestParts::new because it is pub(crate), so should we change to pub?
Or there is a better solution, please tell me. 🙂

@davidpdrsn
Copy link
Member

I think we could look into making RequestParts usable outside axum.

Would have to think a bit more about the design however. new is pretty straight forward but I imagine users would want into_request as well. That is currently not very idiomatic since it takes &mut self (into_* methods normally take self) and panics if the request body has been extracted (to avoid requiring B: Default which might be problematic).

Or maybe there is a totally different way of making extractors usable from services that don't require these bits being public 🤔

If you wanna ship something in async-graphql, and don't want to wait for this, I would probably recommend writing a service without using an extractor for axum.

@davidpdrsn davidpdrsn added the C-enhancement Category: A PR with an enhancement label Aug 7, 2021
@davidpdrsn
Copy link
Member

I was able to clean up the API of RequestParts in #167. I think we could consider exposing the API as it is now.

@sunli829
Copy link
Contributor Author

sunli829 commented Aug 9, 2021

So we only need to change pub(crate) to pub now? 🙂

@davidpdrsn
Copy link
Member

Pretty much 😅 I still wanna do some experimentation and see what its actually like to use.

@davidpdrsn davidpdrsn added this to the 0.2 milestone Aug 16, 2021
@davidpdrsn
Copy link
Member

I've been thinking about this and actually think its fine to make the methods pub. Just requires writing some docs.

@davidpdrsn davidpdrsn added E-easy Call for participation: Experience needed to fix: Easy / not much E-help-wanted Call for participation: Help is requested to fix this issue. labels Aug 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: A PR with an enhancement E-easy Call for participation: Experience needed to fix: Easy / not much E-help-wanted Call for participation: Help is requested to fix this issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants