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

Add support for subscriptions in the GraphQL plugin #249

Merged
merged 2 commits into from Aug 22, 2018

Conversation

clayne11
Copy link
Contributor

Fixes #248.

@clayne11
Copy link
Contributor Author

It looks like something is wrong with the test set up. Node 4 is failing to due to engine incompatibility.

@rochdev
Copy link
Member

rochdev commented Aug 20, 2018

Node 4 tests are broken at the moment because of one of the supported libraries that dropped support for Node 4. We will have to add a way to disable tests for such libraries. For now feel free to ignore Node 4 tests specifically. I'll make sure that Node 4 is not actually broken before releasing new versions.

One question that I have about subscriptions, is do they really work the same as queries and mutations? For example, queries and mutations use the request/response pattern, so they have an explicit start and end. Are subscriptions handled in the same way? What are the start and end times for a subscription?

@clayne11
Copy link
Contributor Author

clayne11 commented Aug 20, 2018

They definitely work a bit differently. The initial call for a subscription sets up the subscription, which uses a call / response pattern.

After that, when the triggering event occurs on the server, the query that the client passed in the subscription set up is run and that data is returned to the client.

It probably shouldn't be measured in exactly the same way as queries / mutations, but adding subscription into the types array definitely solved the error I was seeing.

@rochdev rochdev added bug Something isn't working enhancement New feature or request community integrations labels Aug 20, 2018
@rochdev
Copy link
Member

rochdev commented Aug 20, 2018

So if I understand correctly, for the initial call, the behavior is the same as queries and mutations, so for that call it would be correct to simply add the type? Then we can add support for subscription events in a later version.

@clayne11
Copy link
Contributor Author

I think that sounds reasonable.

@rochdev
Copy link
Member

rochdev commented Aug 20, 2018

Ok, I'll merge the PR as soon as the required tests are passing.

@rochdev rochdev merged commit 06564e7 into DataDog:master Aug 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working community enhancement New feature or request integrations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants