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

Update GraphQL API to be compatible with the graphql-transport-ws protocol #37

Closed
rjwills28 opened this issue Jul 14, 2022 · 12 comments
Closed
Labels
enhancement New feature or request

Comments

@rjwills28
Copy link
Collaborator

Coniql currently uses the tartiflette API to implement the GraphQL server. This uses the deprecated subscription-transport-ws protocol 'graphql-ws'. GraphQL clients are encouraged to move from subscription-transport-ws to graphql-ws (see DiamondLightSource/cs-web-lib#13). The graphql-ws library uses the 'graphql-transport-ws' protocol, which is not supported by tartiflette, therefore we need to move to a different API.

@rjwills28
Copy link
Collaborator Author

I have had a look at some options. There seem to be very few that support the new graphql-transport-ws protocol. Perhaps this will change is people move away from subscription-transport-ws. There also seem to be 2 types of GraphQL APIs - code-first and schema-first. The current Coniql (tartiflette) implementation uses schema-first - i.e. we define a schema in sdl format. Code-first APIs instead write the scheme into the code. It is unclear to me which way is best (there is plenty of discussion about this on the internet). All I can say is that it will potentially be a bigger job to convert to code-first as we will have to convert the schema sdl files into python.

Here are some options:

Code-first:

  • Graphene
  • Strawberry - I've actually started playing with this one trying to get a Query through Coniql working. No luck yet but this does seem quite well documented and supported.

Schema-first

@coretl
Copy link
Contributor

coretl commented Jul 14, 2022

What we have used elsewhere:

@coretl
Copy link
Contributor

coretl commented Jul 14, 2022

Code-first conversion will be fine as we are moving in this direction for other projects, and we can delete half the schema (the device bits) as we do that

@coretl
Copy link
Contributor

coretl commented Jul 14, 2022

@callumforrester may also be interested in this...

@AlexanderWells-diamond
Copy link
Contributor

I'm also interested, as I have a WebUI that talks to Coniql as well as my own webserver, so will have to migrate at the same time as Coniql does.

@coretl
Copy link
Contributor

coretl commented Jul 14, 2022

@rjwills28
Copy link
Collaborator Author

I have been playing around with Strawberry and have created a test server with some of the basic Coniql abilities, namely querying and subscribing to a PV value.

This is available here if anyone wants to take a look at how it works: https://github.com/rjwills28/coniql_strawberry.

One great thing about Strawberry is that it supports both the old and new protocols (graphql-ws and graphql-transport-ws).

I can run it against cs-web-proto by modifying the client subscription to only ask for the value field and using the old protocol. Next I will see if I can get cs-web-proto running with the new protocol.

@rjwills28
Copy link
Collaborator Author

A quick note about web clients:

@callumforrester
Copy link
Contributor

@DiamondJoseph may also have input on web clients.

@DiamondJoseph
Copy link

Altair was what we settled on when doing exactly the same switch of protocols. The browser plugin is convenient, simple integration with express middleware, which was what our frontend was, but your mileage may vary.

@rjwills28
Copy link
Collaborator Author

I found an issue in the Strawberry source code which leads to a memory leak when unsubscribing from PVs updating at high rates. Please see this issue for details: rjwills28/coniql_strawberry#2

@aawdls aawdls added the enhancement New feature or request label Feb 14, 2023
@rjwills28
Copy link
Collaborator Author

Done in move to use the Strawberry GraphQL API, which is compatible with both old and new websocket protocols (PR #44).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants