Add subscriptions #280

kinow · 2019-10-07T01:09:53Z

Closes #207
Closes #266 (noticed this while testing this PR, will add a comment where it fixes this issue)

This pull request adds subscriptions to Cylc UI. The ApolloClient has a feature called link, which defines the transport link. It can be an HTTP link, a WebSockets Link, a mocked link, or whatever transport link you need to transport the data.

We were using only an HTTPLink in the ApolloClient. Now the link is actually a split function (from apollo-link module).

This function acts as a ternary operator, and will decide based on the query definition which link to use. If the query definition specifies it is a subscription, it will use a WebSocketLink (from new package added in this PR apollo-link-ws), otherwise it will use the existing ApolloLink (from apollo-link-http).

Unit tests passing on my environment, managed to fix IDE issues, now it is only missing to make it work with live workflow.

Sibling PRs:

codecov-io · 2019-10-07T01:39:45Z

Codecov Report

Merging #280 into master will increase coverage by 1.52%.
The diff coverage is 70%.

@@            Coverage Diff             @@
##           master     #280      +/-   ##
==========================================
+ Coverage   66.75%   68.28%   +1.52%     
==========================================
  Files          24       16       -8     
  Lines         361      227     -134     
  Branches       20        0      -20     
==========================================
- Hits          241      155      -86     
+ Misses        113       72      -41     
+ Partials        7        0       -7

Impacted Files	Coverage Δ
src/main.js	`0% <ø> (ø)`	⬆️
src/services/workflow.service.js	`0% <0%> (-80%)`	⬇️
src/services/gquery.js	`55.55% <100%> (-12.57%)`	⬇️
src/utils/graphql.js	`66.66% <66.66%> (-33.34%)`	⬇️
src/utils/tasks.js	`83.33% <0%> (-16.67%)`	⬇️
src/store/app.module.js	`100% <0%> (ø)`	⬆️
src/model/User.model.js	`100% <0%> (ø)`	⬆️
src/router/index.js	`0% <0%> (ø)`	⬆️
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4917626...e318e3a. Read the comment docs.

kinow · 2019-10-07T03:07:02Z

Build is passing, tests passing fine. Spent most of the morning working around dependencies that needed to be imported or removed, then rest of morning and part of afternoon fixing tests.

Now the application is loading fine, but the GScan.vue view when accessed - first place where Vue.js will actually try to use WebSockets - is failing with:

Not sure why. Could be because we are using the http protocol instead of ws (the WebSockets are being served by Tornado, which I think is expecting it to be http). But having graphiql working helps a little as we know it should be possible to talk from this browser to the server. 4PM, not likely to find a solution today, but it may be working tomorrow morning and the PR would be ready for review (this is the last part required to update the Vuex data structure via WebSockets)

kinow · 2019-10-07T03:24:25Z

Oh, actually solved the problem. I think it's about 90% done now.

The first issue was that I had to use ws:// as in the GraphiQL template (extremely helpful, whew).

The second issue was that looks like the ApolloClient doesn't use the URI from the links... if you don't specify a URI for the client, when using the HTTP Link it will create a dummy URL simply appending /graphql to the current server URL.

Both fixed, now needs just a modification in the WorkflowService to remove the polling and, instead, use WebSockets with a subscription query. Will start working on that tomorrow morning with ☕ and more calm.

kinow · 2019-10-07T22:16:15Z

Apparently done. Happy to take any reviews now, but can't remove the Draft mark, as this PR if merged without the WebSockets on the backend, it would break Cylc UI.

For me it looks much faster now with WebSockets, and the application is more interactive for users. So if curious, have a try. Hopefully it will work well as long as the other 2 PR's for backend are checked out and built in a venv 🤞

kinow · 2019-10-08T04:17:41Z

package.json

+    "apollo-client": "^2.6.4",
+    "apollo-link": "^1.2.13",
+    "apollo-link-http": "^1.5.16",
+    "apollo-link-ws": "^1.0.19",


These were dependencies of ApolloBoost, a helper that simplifies creating Apollo clients. However, when you provide a link that can handle both HTTP and WebSocket, ApolloBoost ignores it and unless you provide a URI, it will create and use an HTTP Link, simply ignoring the WebSocket.

https://github.com/apollographql/apollo-client/blob/9c19c5810a8908d7034a384d06fdb76eec02b3b6/packages/apollo-boost/src/index.ts#L164

Where it shows that ApolloBoost is not compatible with WebSockets. Hence the removal, and the need to add these dependencies.

kinow · 2019-10-08T04:26:48Z

package.json

    "axios": "^0.19.0",
    "babel-eslint": "^10.0.3",
    "chai": "^4.2.0",
    "cross-env": "^5.2.0",
+    "cross-fetch": "^3.0.4",


We were using the node-fetch for tests, but it didn't work after removing the default ApolloClient (which would prevent Apollo and its libraries of trying to validate that the fetch global existed. This polyfill provides the global fetch for tests.

src/services/workflow.service.js

kinow · 2019-10-08T04:31:06Z

src/services/workflow.service.js


-export { workflowService }
+export { LiveWorkflowService, SubscriptionWorkflowService }


We were exporting a singleton, that once imported would exist for the whole app.

Now we export classes, let users decide whatever they want to do with these (mock, test, extend, instantiate, create a singleton, etc).

Vue has a way to create application globals via Vue.prototype.$applicationSingleton, which has effectively the same result as exporting a singleton, with more flexibility if necessary.

Plus, it will be available to the whole application as this.$workflowService. The $ is per convention.

ps: must be used with care this global objects in Vue, as they docs state

Can we get rid of the LiveWorkflowService now?

I can't recall why I left. Not sure if it was still being used for testing/mocking, or if I was scared that the WebSockets could fail and we would have to revert to the old service 😄 will take another look to confirm.

kinow · 2019-10-08T04:31:47Z

src/utils/graphql.js

-    operation.setContext({
-      headers: {
-        // FIXME: this is the random generated password, update every time it is generated for now!
-        Authorization: 'Basic Y3lsYzpiaFRTbnRpWjRqR2YzSWR3eTlVMg=='


From the old prototype, I suspect the PR for Cylc UI Server auth may require something like this, but we can cross that bridge when time comes, no need to leave an old Cylc Suite passphrase hard-coded here.

kinow · 2019-10-08T04:32:20Z

src/utils/graphql.js

+    }
+  }) : new ApolloLink() // return an empty link, useful for testing
+
+  const link = split(


Think of split as a ternary ? operator. Given a condition, return either the next value, or the subsequent.

kinow · 2019-10-09T00:26:34Z

Updated with one more commit for changelog.

kinow · 2019-10-12T08:00:47Z

CHANGES.md

@@ -24,9 +24,12 @@ workflows only.
 [#283](https://github.com/cylc/cylc-ui/pull/283) - Load user information
 on application startup.

-[#283](https://github.com/cylc/cylc-ui/pull/285) - Update Vuetify to 2.1,
+[#285](https://github.com/cylc/cylc-ui/pull/285) - Update Vuetify to 2.1,


kinow · 2019-10-12T08:02:44Z

Rebased, fixed conflicts, and added one small fix during rebase for the changelog entry of #285 (mea culpa).

kinow · 2019-10-12T08:44:42Z

And fixed something I hadn't tested. I tested the subscriptions with live data. But after rebasing tried the offline mode and it failed. It was because I coded the new import, which supports mocking services, with the SubscriptionWorkflowService class.

Now, instead, we export everything as before, plus an export default WhateverName. Then in the main.js we can do import MyFavouriteName from 'workflow-service'.

In other words, user/caller can use whatever name s/he wants, as the module/callee is exporting a default implementation. SubscriptionWorkflowService for live data, and MockWorkflowService for the offline mode.

Tested both just now after pushing the last changes 👍

kinow · 2019-12-03T21:51:26Z

Huh, I thought it was ready after the PR's of David, but this actually is waiting on another PR of cylc-uiserver: cylc/cylc-uiserver#82

hjoliver · 2019-12-04T00:30:11Z

Merged 😉 (cylc/cylc-uiserver#82)

kinow · 2019-12-04T00:31:24Z

I'm syncing all branches, will do a quick smoke test with this PR again. If everything works fine, will change it to ready for review. Thanks @hjoliver !

kinow · 2019-12-04T01:13:33Z

Re-creating the virtual environments after syncing the repositories, got the following dependency error.

Successfully built cylc-flow
ERROR: graphql-relay 2.0.0 has requirement graphql-core<3,>=2.2, but you'll have graphql-core 3.0.0 which is incompatible.
ERROR: graphene 2.1.8 has requirement graphql-core<3,>=2.1, but you'll have graphql-core 3.0.0 which is incompatible.

It looks to me like there is a typo in the dependency requirement of graphql-ws (the typo is in their setup.py). But I will need to test it.

kinow · 2019-12-04T01:14:23Z

I think this is valid for Conda packages, not sure if for setuptools/pip, where I think you need a comma between version ranges.

kinow · 2019-12-04T01:22:48Z

PR submitted graphql-python/graphql-ws#39

kinow · 2019-12-04T02:40:40Z

Rebased, fixed conflicts, updated the other components & views (graph & gscan were updated after this PR was created).

Also tested with no workflows, with five, and then with five & families2 simultaneously.

Found one problem. Independent of the running of running workflows. I had just five running, but now the Tree component was flickering. Disappearing, and then reappering. Quite annoying.

Looks like there are two messages received at the same time. My suspicion is that GScan is creating one query, and then the Tree view is creating another one. But the Tree view is - for some reason - reacting to both.

The GScan query won't have the familyProxies, which is required for the Tree component, resulting in a moment when the Tree component ignores the data updated in Vuex store. Will investigate a bit more, but very close to marking as ready for review.

kinow · 2019-12-04T08:24:51Z

Ok, just finished with unit tests, and also some quick tests. Tomorrow will test it again with NIWA's notebook after re-creating the whole environment, and if it works there too, it will be ready for review.

kinow · 2019-12-04T08:25:28Z

(One problem was that it had two queries, as I didn't call .unsubscribe on an Observable's subscription. Tricky to find that issue.)

kinow · 2019-12-04T20:39:02Z

Apparently working OK on both my work & home environments. Used two workflows to test it. Found only one browser console message, but that it related to the workflow data structure returned, and how we are accessing attributes and creating the hierarchy (i.e. not related to this issue).

So ready for review. Please, remember that it is required to use the latest Cylc UI Server, which should also install the latest Cylc Flow (or install manually if you prefer to avoid the local cache).

Thanks
Bruno

…instead of exporting a singleton

…ollo instance

kinow · 2019-12-19T23:53:59Z

If GitHub action passes, then this PR is ready for review. Everything worked locally, tested with 2 workflows, also tested mutations. The toolbar buttons are a bit buggy at the moment, some times changing the enable/disable attribute incorrectly. But that is for another ticket.

dwsutherland

From a functional stand-point this PR is ready to go IMO! 👍
(any further improvements can probably be separate PRs)

kinow · 2019-12-20T03:30:14Z

Thanks @dwsutherland !

oliver-sanders

Subscriptions code looks good, websockets in and tested against multiple workflows, nice code improvements, loading lag removed. All good!

The exciting moment seeing websockets show up in the console:

kinow added this to the 0.2 milestone Oct 7, 2019

kinow self-assigned this Oct 7, 2019

This was referenced Oct 7, 2019

Add subscriptions cylc/cylc-uiserver#82

Merged

Add subscriptions cylc/cylc-flow#3390

Merged

kinow force-pushed the add-subscriptions branch from e9f9e43 to 4489d20 Compare October 7, 2019 01:14

kinow commented Oct 8, 2019

View reviewed changes

kinow mentioned this pull request Oct 8, 2019

Invoke mutations to hold, release, and stop workflows #261

Closed

kinow force-pushed the add-subscriptions branch from 5cfdef1 to 806afff Compare October 9, 2019 00:26

kinow force-pushed the add-subscriptions branch from 806afff to 9494c90 Compare October 12, 2019 08:00

kinow commented Oct 12, 2019

View reviewed changes

kinow mentioned this pull request Oct 28, 2019

graph with live data and node info #289

Merged

6 tasks

kinow force-pushed the add-subscriptions branch from 1bc12af to 54a2f1d Compare December 4, 2019 02:23

kinow marked this pull request as ready for review December 4, 2019 20:36

Bruno P. Kinoshita and others added 23 commits December 20, 2019 12:40

Remove Apollo from application plugins

bd8426c

Simplify URL creation

3097610

Specify uri for ApolloClient

63649b6

Add SubscriptionWorkflowService and instantiate a singleton globally …

c8c669b

…instead of exporting a singleton

Use ApolloClient instead of ApolloBoost (split issue)

f9a6a49

Update queries in views to

e8d8519

Removing Apollo Boost (not used anymore)

5094fa0

Add missing dependencies (were transitive from apollo-boost)

4b024d3

Add changelog

87cfad7

Use a default export to trick import for different environments

6ed0d93

Fix lock file after rebase/merge

1e7f617

Use global workflowService from graph and gscan components

da4a649

Use subscription queries in graph and gscan

56e44b6

Await on Vue.nextTick for DOM changes in test

b15fa91

Update test to have a workflowService in the localVue instance

ed164cf

Call destructor method when subscribing

2f4f79f

Add TODO about window.location dependency

469879d

Update Dashboard to use the new service

a9fa099

Remove old LiveWorkflowService and the request method from GQuery

5551d5c

vuex objects must be plan object as vue data

c1696b2

Simplify the observable creation and use

09d16a9

Use the new ApolloClient (with two links) for the toolbar mutations

d0cac5f

Update Toolbar test to use the created apolloClient instead of vue-ap…

e318e3a

…ollo instance

kinow force-pushed the add-subscriptions branch from 8dca002 to e318e3a Compare December 19, 2019 23:49

dwsutherland approved these changes Dec 20, 2019

View reviewed changes

oliver-sanders approved these changes Dec 23, 2019

View reviewed changes

oliver-sanders merged commit 6dc38f5 into cylc:master Dec 23, 2019

kinow deleted the add-subscriptions branch March 19, 2020 01:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add subscriptions #280

Add subscriptions #280

kinow commented Oct 7, 2019 •

edited

Loading

codecov-io commented Oct 7, 2019 •

edited

Loading

kinow commented Oct 7, 2019

kinow commented Oct 7, 2019

kinow commented Oct 7, 2019

kinow Oct 8, 2019

kinow Oct 8, 2019

kinow Oct 8, 2019

kinow Oct 8, 2019

oliver-sanders Dec 18, 2019

kinow Dec 18, 2019

kinow Oct 8, 2019

kinow Oct 8, 2019

kinow commented Oct 9, 2019

kinow Oct 12, 2019

kinow commented Oct 12, 2019

kinow commented Oct 12, 2019

kinow commented Dec 3, 2019

hjoliver commented Dec 4, 2019 •

edited

Loading

kinow commented Dec 4, 2019

kinow commented Dec 4, 2019

kinow commented Dec 4, 2019

kinow commented Dec 4, 2019 •

edited

Loading

kinow commented Dec 4, 2019

kinow commented Dec 4, 2019

kinow commented Dec 4, 2019

kinow commented Dec 4, 2019

kinow commented Dec 19, 2019

dwsutherland left a comment •

edited

Loading

kinow commented Dec 20, 2019

oliver-sanders left a comment •

edited

Loading


		export { workflowService }
		export { LiveWorkflowService, SubscriptionWorkflowService }

Add subscriptions #280

Add subscriptions #280

Conversation

kinow commented Oct 7, 2019 • edited Loading

codecov-io commented Oct 7, 2019 • edited Loading

Codecov Report

kinow commented Oct 7, 2019

kinow commented Oct 7, 2019

kinow commented Oct 7, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kinow commented Oct 9, 2019

Choose a reason for hiding this comment

kinow commented Oct 12, 2019

kinow commented Oct 12, 2019

kinow commented Dec 3, 2019

hjoliver commented Dec 4, 2019 • edited Loading

kinow commented Dec 4, 2019

kinow commented Dec 4, 2019

kinow commented Dec 4, 2019

kinow commented Dec 4, 2019 • edited Loading

kinow commented Dec 4, 2019

kinow commented Dec 4, 2019

kinow commented Dec 4, 2019

kinow commented Dec 4, 2019

kinow commented Dec 19, 2019

dwsutherland left a comment • edited Loading

Choose a reason for hiding this comment

kinow commented Dec 20, 2019

oliver-sanders left a comment • edited Loading

Choose a reason for hiding this comment

kinow commented Oct 7, 2019 •

edited

Loading

codecov-io commented Oct 7, 2019 •

edited

Loading

hjoliver commented Dec 4, 2019 •

edited

Loading

kinow commented Dec 4, 2019 •

edited

Loading

dwsutherland left a comment •

edited

Loading

oliver-sanders left a comment •

edited

Loading