Implementation for juniper_actix #603
Conversation
engylemure
force-pushed
the
juniper_actix_subscriptions
branch
7 times, most recently
from
58f6249 to
35c0bc2
Compare
There was a problem hiding this comment.
To fix CI you need to add juniper_actix to
Line 25 in c12f40c
juniper_subscriptions/src/lib.rs
Outdated
| @@ -135,6 +135,94 @@ where | |||
| } | |||
| } | |||
|
|
|||
| /// Subscriptions Protocol Message Types | |||
| /// to know more access https://github.com/apollographql/subscriptions-transport-ws/blob/master/PROTOCOL.md | |||
| pub mod message_types { | |||
juniper_subscriptions/src/lib.rs
Outdated
| // in the moment im considering that this value is | ||
| // the first text message sent by the Client with the | ||
| // type GQL_CONNECTION_INIT | ||
| connection_params: &str, |
There was a problem hiding this comment.
I'd prefer to not have it there if it is not defined. If there are specific params, we should do an enum with an ::Other(String).
juniper_subscriptions/src/lib.rs
Outdated
| /// Empty SubscriptionLifeCycleHandler | ||
| pub struct EmptySubscriptionLifecycleHandler {} | ||
|
|
||
| impl EmptySubscriptionLifecycleHandler { |
There was a problem hiding this comment.
This should be deleted and just derive Default.
juniper_subscriptions/src/lib.rs
Outdated
| // type GQL_CONNECTION_INIT | ||
| connection_params: &str, | ||
| context: &mut Context, | ||
| ) -> Result<(), String>; |
There was a problem hiding this comment.
This should be an error enum rather than a string.
There was a problem hiding this comment.
What kind of values this enum should have?
juniper_subscriptions/src/lib.rs
Outdated
|
|
||
| /// Trait based on the SubscriptionServer | ||
| /// https://www.apollographql.com/docs/graphql-subscriptions/lifecycle-events/ | ||
| pub trait SubscriptionLifecycleHandler<Context> |
There was a problem hiding this comment.
I feel like this should be a state machine? Also on_ functions don't feel idiomatic...not sure what the best way to handle it tough.
There was a problem hiding this comment.
I made some changes but since I doesn't have much experience into how this state machine should be implemented a example should be of great help to me.
engylemure
force-pushed
the
juniper_actix_subscriptions
branch
from
711874c to
0a821a2
Compare
juniper_subscriptions/src/lib.rs
Outdated
| @@ -135,6 +136,108 @@ where | |||
| } | |||
| } | |||
|
|
|||
| /// Enum of Subscription Protocol Message Types | |||
There was a problem hiding this comment.
Hmmmm, it looks like this is a non-standard extension right? The only standard I see is http://spec.graphql.org/draft/#sec-Subscription
There was a problem hiding this comment.
Yeah, it makes sense, it is specific to the implementation of subscriptions over WS.
|
Would it be too much trouble to pull actix-web stuff out of this and into another PR so we can look at the subscription changes on their own? |
No, i made another PR with only these changes #617 and merged this PR into it. |
|
It looks like the |
engylemure
force-pushed
the
juniper_actix_subscriptions
branch
from
4979397 to
10dadd7
Compare
Sure |
engylemure
changed the title
|
Thank you again, this looks great. The community has really wanted this! Also, appreciate you making so many changes to the PR 🍻. |
This PR implements the integration with actix-web 2.0 and has a initial study on the implementation of the GraphQL subscriptions int othe actix as well, i made some small addition in the juniper_subscriptions as well a submodule for storing the message types sent between the client and server, added a trait as well for handling some lifecycle events in the subscription such as on_connection and some other ones, since right now the implementation depends on a unsafe operation for sending messages this is just a alpha implementation and would like to receive some reviews on it.
I made some tests and examples as well for handling all things that were implemented.
PR Checklist
Before submitting a PR, you should follow these steps to prevent redundant churn or CI failures:
Error in CI
for some reason my CI is not working and it is being caused by the juniper_rocket crate requiring the nightly build, the tests in the nightly are running just fine in the stable they are just breaking...