Irmin graphql subscriptions

[andreas]

⚠️ Experimental! Depends on andreas/ocaml-graphql-server#133 ⚠️

This PR updates irmin-graphql to use graphql-cohttp. This change should happen regardless, as graphql_lwt no longer includes a HTTP server. (no longer necessary)

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...

• Pending the release of graphql 0.9.0 (PR)

/cc @zshipko

[zshipko]

This looks great!

@samoht just merged my graphql-cohttp integration branch so there are a bunch of conflicts, but the subscription related code should definitely be included ASAP.

We're preparing for an irmin release soon, so will have to hold off on merging this until the upstream changes have been published to opam. Definitely keep us updated as things progress, but I will also watch out for the cohttp and websocket-lwt releases!

[andreas]

@samoht just merged my graphql-cohttp integration branch so there are a bunch of conflicts, but the subscription related code should definitely be included ASAP.

Ah, great -- I'll rebase 😄

We're preparing for an irmin release soon, so will have to hold off on merging this until the upstream changes have been published to opam. Definitely keep us updated as things progress, but I will also watch out for the cohttp and websocket-lwt releases!

Sure, will do. Merging this depends on both a new release of cohttp and graphql-cohttp, but I'll keep you posted.

[andreas]

I've updated this PR to be the minimal patch to support websocket based subscriptions based on andreas/ocaml-graphql-server#133.

While making these changes, I noted that having run_server as part of Irmin_graphql.S causes a lot of the complexity. If instead Irmin_graphql.S only allowed creating a value of type Cohttp_lwt.S.Server given a store, then I think a lot of the complexity would go away. This is essentially what I did in this commit 1733668, which was part of the previous version of this PR. What are your thoughts on that approach? 😄

[zshipko]

Thanks!

Yeah, I was looking at your code and like how simple the interface is using only Cohttp_lwt.S.Server rather than the custom SERVER type. That approach seems much less complicated! I am happy to make the changes needed to remove SERVER and create a new PR to review it. How does that sound?

[andreas]

I've rebased on #599 now.

[andreas]

graphql et al v0.9.0 has now been released on OPAM, so this PR is ready for review/merge 🎉

Travis passes for irmin-graphql passes, but irmin-unix is failing for a seemingly unrelated reason:

#=== ERROR while compiling irmin-unix.dev =====================================#
# context     2.0.3 | linux/x86_64 | ocaml-base-compiler.4.06.1 | pinned(file:///home/travis/build/mirage/irmin)
# path        ~/.opam/ocaml-base-compiler.4.06.1/.opam-switch/build/irmin-unix.dev
# command     ~/.opam/opam-init/hooks/sandbox.sh build dune build -p irmin-unix -j 1
# exit-code   1
# env-file    ~/.opam/log/irmin-unix-16226-ffb3fd.env
# output-file ~/.opam/log/irmin-unix-16226-ffb3fd.out
### output ###
#     ocamlopt src/irmin-unix/bin/main.exe (exit 2)
# (cd _build/default && /home/travis/.opam/ocaml-base-compiler.4.06.1/bin/ocamlopt.opt -w -40 -g -o src/irmin-unix/bin/main.exe -I /home/travis/.opam/ocaml-base-compiler.4.06.1/lib/angstrom -I /home/travis/.opam/ocaml-base-compiler.4.06.1/lib/astring -I /home/travis/.opam/ocaml-base-compiler.4.06.1/lib/base -I /home/travis/.opam/ocaml-base-compiler.4.06.1/lib/base/caml -I /home/travis/.opam/oca[...]
# File "_none_", line 1:
# Error: Files /home/travis/.opam/ocaml-base-compiler.4.06.1/lib/digestif/ocaml/digestif_ocaml.cmxa
#        and /home/travis/.opam/ocaml-base-compiler.4.06.1/lib/digestif/c/digestif_c.cmxa
#        both define a module named Digestif_bi

/cc @samoht @zshipko

[samoht]

/cc @dinosaure seems that we are double-linking digestif.

[samoht]

@andreas is there any library where we are using digestif.ocaml and digestif.c while we were supposed to use only digestif ?

[andreas]

Ah this makes sense. I've specified digestif.ocaml as a dependency in graphql-cohttp 😞 I'll update it..

Thanks!