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

Make subscriptions work #132

Closed
wants to merge 2 commits into from
Closed

Conversation

caseymrm
Copy link
Contributor

There's some code towards supporting graphql subscriptions in queries, this fixes a few spots where they weren't handled yet.

@ulm0
Copy link

ulm0 commented Dec 17, 2017

Any updates on merging this?

@andrewshvv
Copy link

@caseymrm Hey! Glad to see that someone made a movement towards subscription implementation! 🍾 Is it possible to use them after merging the pull request and do you have an example of it?

@kivutar
Copy link

kivutar commented Jan 3, 2018

This should be merged. It is a step forward in supporting subscriptions.
And the next step would be to have a WebSocket implementation like this one https://github.com/functionalfoundry/graphqlws

@robermorales
Copy link

I heard at #88 that @neelance held their dedication on the project. First of all, thanks all, and specially thanks to @neelance for the work on this pretty cool library.

Any idea on how the development can be directed now? Are the new PRs going to be revised and eventually accepted in this repo? Should somebody create a fork to lead the project from another repo?

@neelance
Copy link
Collaborator

neelance commented Jan 3, 2018

I am not opposed to a fork. Like I mentioned on #88, when at some point I have time available to spend on this project, then it is likely to undergo some major changes. That's maybe also not what everyone wants. If there is a good fork at some point, then I may link to it from the readme.

On this PR: Is this change already helpful on its own? Without WebSockets, subscriptions won't be "live", the only thing you could do is normal one-time queries on endpoints called "subscription". I am not yet sure that it is a good idea to add a half-baked state of the subscriptions feature.

@kivutar
Copy link

kivutar commented Jan 3, 2018

If the websocket part is handled by another module like done with graphql-go+graphqlws, this present change should be enough for this repo.
A second change may be needed later in graphiql to include a websocket client.

@caseymrm
Copy link
Contributor Author

caseymrm commented Jan 3, 2018

Yeah, to be clear, this just promotes subscriptions to work the same as query or mutation. So it's purely semantics, until this is merged I currently do my subscriptions as queries and the rest of the code is unchanged. You could say the same thing about mutation though- technically you could just use a query for everything, but I like the separation.

Overall, your library already has some of the code to parse subscription, so it seems worth finishing it enough to make them usable. The library does not handle transport mechanisms in general, so I think this can go in without websocket support. In go it's so trivial to do a long poll (my code just does a blocking read on a channel inside the resolver) that I'm just doing that for now.

But up to you of course, feel free to close, or if there's additional work that you think would make this more baked I'm happy to take a look.

@paulxuca
Copy link

paulxuca commented Jan 5, 2018

There's been some work at a fork https://github.com/mikeifomin/graphql-go/tree/subscription

@mikeifomin
Copy link

@paulxuca yeah. it's still in progress
My idea is to use channels in resolvers
https://github.com/mikeifomin/graphql-subscriptions-go-example/blob/master/schema/resolver.go#L42

Does anyone have any better ideas?

@tonyghita
Copy link
Member

tonyghita commented Jan 6, 2018

Have you watched Lee Byron's introduction to GraphQL subscriptions? https://www.youtube.com/watch?v=bn8qsi8jVew

I don't think a purely channel-based solution would be flexible enough for the purposes of this library.

@mikeifomin
Copy link

mikeifomin commented Jan 6, 2018

@tonyghita yes I saw it.
I've implemented apollo ws server protocol. (mikeifomin@b407fab)
With manual browser testing on a simple subscriptions it works.

The high level idea is to add method

func (s *Schema) Subscribe (...) <-chan *Response

like

func (s *Schema) Exec (...) *Response

Im really serious about subscription with go. I'm already using graphQL subscription on my project and have mocked server with node.js.
@tonyghita can you give me a little more details about your idea of flexibility?

@tonyghita
Copy link
Member

tonyghita commented Jan 7, 2018

can you give me a little more details about your idea of flexibility?

It sounds like you've put more practice into subscriptions + go than I have, but the biggest questions for me are:

  • Can the subscription implementation be agnostic of transport (output)?
    In my ideal library, I could plug in a WebSocket transport or WebSub transport, both, or something else (Server-Sent Events?). Just like this library doesn't dictate that you have to use HTTP, I don't think it should dictate the subscription transport.

  • Can the subscription implementation be agnostic of event system (input)?
    We should probably be able to specify go channels, or kafka, or whatever mechanism to provide the functionality. I'm not sure it's best that the mechanism should be exposed via the library's interface.

  • Can it scale reasonably?
    Unlike queries and mutations, subscriptions are stateful and as such more difficult to scale horizontally and make robust to failure. Does it make sense to introduce a stateful component to this library? Should that be delegated to a separate component which can be scaled independently of the stateless component? (I understood this as a takeaway from Lee's talk)

  • Follow up: if the query/mutation and subscription components are separate, what is the interface between them?

@kivutar
Copy link

kivutar commented Jan 8, 2018

In the apollo-server stack, the SubscriptionClient interface can be provided by different modules, one of them being websocket https://github.com/apollographql/subscriptions-transport-ws

And the PubSub mechanism is also abstracted, so you can use the default one, or redis, or something else https://github.com/davidyaha/graphql-redis-subscriptions

Their pubsub interface returns an AsyncIterator that you plug in your resolver.

@jmichalicek
Copy link

I really appreciate the work you guys are putting into this. I fell in love with golang just a few months ago and have been surprised at the lack of GraphQL support. I've got a project using this package I'm working on just to see how it turns out, but the general lack of GraphQL support in the Go community keeps me questioning the wisdom of that when I had a working version in Elixir and could churn one out in Python in a few days.

I'd love to see this project turn into Go's Apollo/Absinthe/Graphene.

@mytototo
Copy link

mytototo commented Mar 6, 2018

Hi there,

Is there any ETA regarding this PR?
Is it possible to update the example with this feature?

Thanks a lot for your contribution.

@matiasanaya
Copy link
Contributor

matiasanaya commented Apr 25, 2018

I've put together a graphQL subscriptions proof of concept that:

  • Relies on a fork of github.com/graph-gophers/graphql-go (which incorporates many of @mikeifomin's ideas and code) as the subscription implementation.
  • Relies on a fork of github.com/functionalfoundry/graphqlws as the transport implementation.
  • Separates subscription implementation from transport implementation as suggested by @tonyghita.
  • Has a subscription implementation that is agnostic of the event system, also suggested by @tonyghita.

I'll be focusing my efforts on the github.com/graph-gophers/graphql-go fork next. What I've got right now is no more than a buggy (working) prototype. I'm keen on input from anyone who's interested.

I've put a PR up.

@matiasanaya
Copy link
Contributor

I've updated the example subscriptions server (which uses this library) with a new websockets transport library. If anyone is keen on using subscriptions please have a look at those projects and maybe contribute :)

@pavelnikolov
Copy link
Member

@caseymrm and @tonyghita I'd be interested to hear your feedback on @matiasanaya's PR.

@pavelnikolov
Copy link
Member

There is a working version of subscriptions now.

@lindroth
Copy link

So I saw! Great work everyone and a big thanks! <3

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.