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 actix web integration #149

Closed
LegNeato opened this issue Feb 21, 2018 · 17 comments
Closed

Add actix web integration #149

LegNeato opened this issue Feb 21, 2018 · 17 comments

Comments

@LegNeato
Copy link
Member

https://github.com/actix/actix-web integration would be swell.

@radix
Copy link

radix commented Feb 21, 2018

#2 is relevant to this, because actix-web is an asynchronous web framework based on tokio.

Of course, it can still be used from a threadpool or whatever, so integration can still be done without #2. But with #2 resolved, the integration could be tighter and avoid requiring additional threads.

@LegNeato
Copy link
Member Author

So I have never used actix, but it looks like the actors can be both sync and async. The diesel example looks like the triggering is async but doing the actual db work is sync?

@radix
Copy link

radix commented Feb 21, 2018

yes @LegNeato. I should have been more clear, actix can definitely deal with blocking stuff (by using threads itself), but it could also make good use of native async support in juniper.

@theduke
Copy link
Member

theduke commented Feb 22, 2018

Yeah, for now we need to implement this with the GraphQL queries being executed on a thread pool, which shouldn't be hard to do (note: I've never used Actix, but wanted to try it out).

Anyone familiar with actix-web who is interested in tackling this?
Like the other integration crates, this should only require a little bit of glue code.

@pyrossh
Copy link

pyrossh commented Feb 26, 2018

Lol.. Just saw this. Was going to post on issue on this but I already did the integration for actix-web.
actix/actix-web#81

Yes @LegNeato Actors can be sync and async. The sync support is done through workers.
I'm still working on one more example to implement juniper without blocking the main thread.
We would need to have a juniper/graphql actor whose sole purpose is executing grapqhl requests and then start an arbiter/monitor who spins up actors that way actix-web can handle juniper's sync nature without a hinch.
On our request handler we would parse the req and send our graphql actor a graphql request and wait until it returns without blocking the main event loop or single thread. The best part is we can have both worlds sync and async and don't have to be restricted to just one.

@thedodd
Copy link

thedodd commented Apr 22, 2018

Yea, we can probably close this issue as the actix-web repo has a pretty solid example of how to integrate with Juiper. Works quite well.

@pyrossh
Copy link

pyrossh commented Apr 22, 2018

Yes.. Please do.

@theduke
Copy link
Member

theduke commented Apr 29, 2018

I have added a link to https://github.com/actix/examples/tree/master/juniper in the README.

Would anyone familiar with actix-web be willing to write up a small guide for the book? (https://github.com/graphql-rust/graphql-rust.github.io(

@jkirkpatrick
Copy link

For anyone finding the issue on this repo, the actix-web juniper demo has a minor issue to support Graphiql. It's missing CORS (gist here, listener port is 8080): https://gist.github.com/jkirkpatrick/bd795a515fe85c2def1c047fc1501ba9

@BrendanBall
Copy link
Contributor

Has anyone actually used juniper with actix-web where the graphql resolvers talk to a db actor? The juniper and actix-web example shows how to run juniper inside actix-web but then basically hardcodes the resolver responses. As @LegNeato mentioned, you can easily create a sync actor with a SyncArbiter but communicating with that actor is still wrapped in Futures. Am I missing something? Combining example https://github.com/actix/examples/tree/master/juniper with example https://github.com/actix/examples/tree/master/diesel to create a more complete integration example would be really awesome. I'm basically trying to do that with my own little toy app that just uses a CHashMap wrapped in an actor as the db, but I'm failing integrating the graphql resolver with the db actor. If anyone is interested in helping I can create a WIP example of combining those 2 actix examples and if it's solvable we can add the example in the juniper or actix list of examples.

@thedodd
Copy link

thedodd commented Dec 5, 2018

@BrendanBall I’m using actix-web with Juniper via sync actor. My resolvers are communicating with gRPC services which are themselves interfacing with DBs.

All you need to do is model your graph types, and then regardless of where your resolver data is coming from (DB, gRPC calls, or somewhere else), just map the data onto your graph types and return it inside of a juniper FieldResult. Or if there is no chance of an error, then your resolvers can just return the plain data without being wrapped in a result type.

Also, you may not need a separate actor for your DB. You could just use the DB interface directly from your resolver, as the sync actor delegates to a thread pool. Juniper is currently not async, so your DB interface does not need to be from within the resolver.

However, if your DB interface is async, then you could spawn your DB futures onto one of the actix tokio reactors and wait for it to resolve in your resolver. You could use an mpsc oneshot to send your future data back to a sync caller (your resolver). There are a few options you could go with.

Let me know if you would like me to go into more depth on one of those options for more info.

@pyrossh
Copy link

pyrossh commented Dec 5, 2018

@BrendanBall I had converted a bit of my rocket web app to actix. I just used a graphql sync actor thats all that's needed. If your db is sync it will resolve properly in the graphql sync actor you won't need to create a db actor on anything. So its alright.
https://github.com/pyros2097/rust-web/tree/actix2

@BrendanBall
Copy link
Contributor

BrendanBall commented Dec 5, 2018

@thedodd @pyros2097 How do you decide whether something should be an actor or not? I would think the "correct" thing to do would be to make your db an actor. In this case you could get away with not making db an actor which would make this very easy, but I'm more trying to evaluate the case where that dependency like db is async or really should be an actor. And in that case how difficult is it to link up a graphql resolver with an actor while juniper is not async yet.

@thedodd As mentioned in my previous comment, I've gone and combined 2 examples (actix juniper and actix diesel) into 1 more integrated example here and I've left the currently broken resolvers temporarily commented out here.

Would you be able to go into more depth on how I would properly implement the commented out resolver? I'm interested in the 2 options

  1. Spawn your DB futures onto one of the actix tokio reactors and wait for it to resolve in your resolver.
  2. You could use an mpsc oneshot to send your future data back to a sync caller (your resolver).
    If you could even create a PR to my example that would be fantastic. If I'm not being crazy trying to do this, then I would be happy to add this example to an official actix/juniper example if others are running into this problem.

PS. I appreciate all the feedback as I'm still pretty new to rust

@thedodd
Copy link

thedodd commented Dec 5, 2018

@BrendanBall so

How do you decide whether something should be an actor or not?

A simple way of looking at this is that Actors are specifically for async message passing. In the world of async IO, performing a synchronous operation from within an async context is a bad deal. This is where Actors come in (there's more to this, but I am simplifying).

Actors allow you to bridge the gap between sync code and async code via message passing. Sync actors get their own background threads to do their sync work on, and then produce their results as async messages (which the actix system manages).

Juniper is synchronous, which is why you need to wrap it in an Actor. Within Juniper resolvers you are in a sync context. So if your DB interface is sync, then there is no need to turn it into an Actor.

I'll take a look at the other things you mentioned here shortly.

@thedodd
Copy link

thedodd commented Dec 5, 2018

@BrendanBall fortunately the issue with your broken resolver is pretty simple.

  1. You're sending the GetUser message to the DB actor (which returns a Future) and then mapping the resolved value into an Actix response type.
  2. This is wrong because it is taking place within a Juniper resolver which expects you to return a FieldResult<User>, not a Future<ActixResponse> (or whatever the exact type is that is being returned there currently).
  3. You should be able to resolve this pretty easily by just using the database interface directly and returning the found User model. You may be able to just to Ok(user) where user is the model returned from the DB. You'll still want to do error handling and such, of course.

Last thing I'll mention on that front is that your GraphQL actor is already taking care of handling the returned data from the juniper executor interface, as seen here. Looks like you are serializing as a JSON string with serde and such. That's good. That is the only location where you should be serializing the Juniper results.

@BrendanBall
Copy link
Contributor

@thedodd thanks for the feedback, that seems to be pretty straightforward when not making db an actor. Maybe by the time I need async like for grpc or something, juniper will be async.

@thedodd
Copy link

thedodd commented May 3, 2019

Sounds like we can probably go ahead and close this issue now?

@LegNeato LegNeato closed this as completed May 3, 2019
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

No branches or pull requests

7 participants