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

Irmin graphql subscriptions (take 2) #616

Merged
merged 1 commit into from
Feb 19, 2019

Conversation

andreas
Copy link
Contributor

@andreas andreas commented Feb 15, 2019

The PR description of andreas/ocaml-graphql-server#141 unintendedly closed #598 with no option of reopening, so this PR is a copy of #598 😒

This PR adds the ability to watch the entire store or a specific key with notifications delivered via the GraphQL subscription mechanism (currently only websockets, possibly Server-Sent Events or chunked HTTP responses in the future).

Adding Irmin_unix.set_listen_dir_hook () to cli.ml may be required...

irmin-graphql-subscriptions-demo-low-res

@andreas andreas force-pushed the irmin-graphql-subscriptions branch from 9ac3919 to cf5a5a5 Compare February 15, 2019 09:34
@andreas
Copy link
Contributor Author

andreas commented Feb 15, 2019

I'm not quite sure why Appveyor cannot find graphql-cohttp.0.10.0, but it works in Travis. Maybe the OPAM repo is cached somehow?

Otherwise, this PR should be ready for review.

@samoht
Copy link
Member

samoht commented Feb 18, 2019

Any idea why the irmin-git tests are failing ?

@zshipko
Copy link
Contributor

zshipko commented Feb 18, 2019

I think the irmin-git test failure is actually unrelated to this PR. I'm getting the same failure as the one on travis-ci when running tests for the code in mirage/irmin#master without any modifications. However, the failure seems to be kind of inconsistent, so I haven't really been able to debug it.

Copy link
Contributor

@zshipko zshipko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great!

One question since I'm not all that familiar with how subscriptions work: it looks like unwatch will be called automatically when the client unsubscribes, but how does the client send the actual unsubscribe request?

src/irmin-graphql/irmin_graphql.ml Show resolved Hide resolved
src/irmin-graphql/irmin_graphql.ml Show resolved Hide resolved
@andreas
Copy link
Contributor Author

andreas commented Feb 19, 2019

One question since I'm not all that familiar with how subscriptions work: it looks like unwatch will be called automatically when the client unsubscribes, but how does the client send the actual unsubscribe request?

The client sends a GQL_STOP message (doc) over the websocket, which will terminate the subscription -- this triggers a call to unwatch.

@andreas andreas force-pushed the irmin-graphql-subscriptions branch from cf5a5a5 to ad937bb Compare February 19, 2019 09:19
@andreas
Copy link
Contributor Author

andreas commented Feb 19, 2019

Any idea why the irmin-git tests are failing ?

I'm not sure either. It's green now without any relevant changes on my part. I did rebase on master though.

@zshipko
Copy link
Contributor

zshipko commented Feb 19, 2019

Any idea why the irmin-git tests are failing ?

I'm not sure either. It's green now without any relevant changes on my part. I did rebase on master though.

Yeah, I'm not sure what's going on with the tests 😕

@zshipko zshipko merged commit cbd5577 into mirage:master Feb 19, 2019
@andreas andreas deleted the irmin-graphql-subscriptions branch April 6, 2019 12:22
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.

3 participants