Add gscan component #301

kinow · 2019-11-05T03:44:45Z

These changes partially address #139

Initial version of the GScan component. Will need more iterations later until the requirements in #139 are fully addressed.

Requirements check-list

I have read CONTRIBUTING.md and added my name as a Code Contributor.
Contains logically grouped changes (else tidy your branch by rebase).
Does not contain off-topic changes (use other PRs for other changes).
Appropriate tests are included (unit and/or functional).
Appropriate change log entry included.
No documentation update required.

kinow · 2019-11-05T03:47:57Z

Note: it takes users to /workflows/$workflow-name. Which at the moment is rendered by the Tree view. Later, we should replace it by a Workflow.vue I think, which adds the initial area to add tabs as in #98

It also does not implement any hierarchy.

kinow · 2019-11-05T03:51:54Z

And a GIF of what it looks like in action:

codecov-io · 2019-11-05T03:56:20Z

Codecov Report

Merging #301 into master will increase coverage by 4.87%.
The diff coverage is 67.74%.

@@            Coverage Diff             @@
##           master     #301      +/-   ##
==========================================
+ Coverage   62.57%   67.44%   +4.87%     
==========================================
  Files          22       24       +2     
  Lines         318      341      +23     
  Branches       17       20       +3     
==========================================
+ Hits          199      230      +31     
+ Misses        115      105      -10     
- Partials        4        6       +2

Impacted Files	Coverage Δ
src/store/workflows.module.js	`66.66% <ø> (-33.34%)`	⬇️
src/router/index.js	`0% <0%> (ø)`	⬆️
src/components/cylc/gscan/index.js	`100% <100%> (ø)`
src/components/cylc/tree/index.js	`100% <100%> (ø)`	⬆️
src/components/cylc/gscan/GScan.vue	`57.89% <57.89%> (ø)`
src/services/gquery.js	`68.11% <0%> (+14.49%)`	⬆️
src/services/workflow.service.js	`80% <0%> (+80%)`	⬆️

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 150a6cd...fc9f14f. Read the comment docs.

kinow · 2019-11-05T03:57:10Z

Build failed due to sass-loader and vuetify. Removing node-sass to see if it will pass now (off to home, will log in again later to check if build passed). Only tested with the workflow five, so it would be good if a reviewer could test with other workflows and see if there's anything broken :)

kinow · 2019-11-05T04:05:11Z

Build passed on Travis 🎉

oliver-sanders

That was quick!

Looks nice, with your last PR merged this is starting to really look like the Cylc GUI now. It's getting pretty usable!

src/App.vue

src/components/cylc/GScan.vue

oliver-sanders · 2019-11-06T14:40:22Z

src/components/cylc/GScan.vue

+            <v-layout align-center align-content-center wrap>
+              <v-flex grow>{{ workflow.name }}</v-flex>
+              <v-flex shrink ml-4>
+                <!-- task summary tooltips -->


Nice.

I think we might want to factor out some of this functionality into a common component so all views can benefit from these features (e.g. tooltips).

I was thinking of something like JobBox which either holds:

State totals (as in this case).

A collection of jobs (which collapses down to state totals if there are too many jobs).

Food for later thought...

@oliver-sanders - can you post an issue for this, if you think it's needed.

oliver-sanders · 2019-11-06T14:41:55Z

src/App.vue

+const QUERIES = {
+  root: `
+    {
+      workflows {


BTW: I think we will need to improve the whole gproxy thing so it can merge this query properly, my bad I should have opened a follow on issue at the time.

There are two or three issues created that involve changes in the gproxy, so there's room for improvement, but it was a life saver. Without that early in the project, we would have a lot built on top of - probably - multiple queries spread over the project.

@oliver-sanders - ditto this - is an issue needed, it will it be obvious later on?

kinow · 2019-11-06T22:23:37Z

Rebased, fixed conflicts. May still have to update the code based on further comments from @oliver-sanders (I think I replied with only more questions hehe)

hjoliver · 2019-11-08T03:02:01Z

@kinow - have you tested this with more than one workflow? When I do, the first one I click on (in the scan bar) appears in the tree view, but then second one results in a blank view. The first view comes back if I click on the first one again in the sidebar.

kinow · 2019-11-08T05:02:18Z

@kinow - have you tested this with more than one workflow? When I do, the first one I click on (in the scan bar) appears in the tree view, but then second one results in a blank view. The first view comes back if I click on the first one again in the sidebar.

Oh! Interesting issue! I surely tested two workflows, five, and families2 (an example you provided in some issue). But I probably did not test both workflows at the same time 😄

Let me try it and see what happens.

kinow · 2019-11-08T07:10:10Z

Ah! Mea culpa! I believe in the families PR I changed the code to build the workflow tree for the first workflow (implemented that for a single workflow, when looking at the /vuews/Tree.vue).

Easy to cover with an e2e test. It might be time to start considering adding more e2e tests, and adding them to Travis/CI. (had added this comment to another issue wrongly, sorry)

I think it's fixed now!

moved the logic of computing the workflow out of Vuex, into the view Tree.vue
fixed the router title problem, which was not displaying the workflow name in the toolbar
tested with two workflows at the same time
- one workflow (families2 source below) occasionally had some exceptions due to a value missin from the lookup table (i.e. a task firstParent was missing from familyProxies). After that, the application wouldn't allow users to switch workflows, being piratically broken. Added a try/catch and will log the exception to the browser console, and allow users to keep using the rest of the app - there may be a moment that the workflow tree is empty due to this exception

kinow · 2019-11-08T07:14:36Z

(just need to fix the tests)

hjoliver · 2019-11-09T09:55:39Z

That's got it, looks great.

hjoliver

Approving this on the basis of functionality (my JS and Vue is still lacking 😬). Looks good, seems to work well, and we can extend and improve it later.

I'd like to get this merged this week to use in demos in the US. @oliver-sanders and @kinow can you figure out between you if all review comments have been addressed or moved to follow-up issues?

Finally, we need some way to access to the cytoscape graph view!

kinow · 2019-11-09T10:28:39Z

Finally, we need some way to access to the cytoscape graph view!

Without #98 there's no way to access the graph now 😞 unless one knows the URL. Do you have any suggestion where we could include a link for the Graph view? Or perhaps we should try to display both components at the same time by default?

hjoliver · 2019-11-10T00:40:49Z

As a temporary measure, could we just keep the old scan table view via a link from the dashboard?

kinow · 2019-11-10T02:16:20Z

As a temporary measure, could we just keep the old scan table view via a link from the dashboard?

Sure. Or should we add that link under the Dashboard entry in the drawer/left sidebar? It just occurred to me that I could have simply added it under the existing values (a bit of d'oh moment). If that's fine, it'll be a 5 min job.

hjoliver · 2019-11-10T09:36:58Z

Yep that’s fine 👍🏻

kinow · 2019-11-10T09:50:04Z

Yep that’s fine 👍🏻

Done (and rebased too)

src/components/cylc/Drawer.vue

kinow · 2019-11-10T21:53:59Z

src/components/cylc/GScan.vue

+import { workflowService } from 'workflow-service'
+
+const QUERIES = {
+  root: `


@oliver-sanders 👆 moved query and methods/data/etc from App.vue to this component

kinow · 2019-11-10T21:56:46Z

src/components/cylc/GScan.vue

+          return 'mdi-pause-octagon'
+        default:
+          return 'mdi-help-circle'
+      }


@oliver-sanders, @hjoliver also added an extra commit to choose the workflow icon. Given we have the icons loaded anyway, and they fit the CSS styles, would it be fine? Or do we need the component with SVG? They don't look exactly like the design sketch, but looks good to me.

(did a cylc hold five to test both icons)

(note that the Workflows menu item is activated as it matches the "/workflows*" route)

The play and pause icons look fine to me, and we can change to custom svg later on if we want to.

hjoliver · 2019-11-11T04:48:09Z

Looking good.

Now wondering if gscan tool-tip reduction is feasible this week (or did that require new data from the back-end? I forget the result of that discussion...).

New:

Old:

kinow · 2019-11-11T05:20:25Z

Now wondering if gscan tool-tip reduction is feasible this week (or did that require new data from the back-end? I forget the result of that discussion...).

I think @oliver-sanders pointed the http endpoint from where we used to get this data from, and in the discussion here and on Riot we were discussing that it would be possible to get state totals from the backend - though I believe it is not the same problem.

I think this is the code in GScan that creates the message from your screenshot. It gets the data from this method from the state_summary_mgr.

I will try to reproduce this in JS ☝️

hjoliver · 2019-11-11T06:12:56Z

Thanks @kinow (and tomorrow is OK!)

…tasks displayed to 5

kinow · 2019-11-11T06:59:50Z

Rebased, and implemented:

sorting the tasks within each state alphabetically (JS, when creating the summary data)
display only N (default to 5) tasks (JS/Vue in the component, we use array.slice(0, N) to get up to N... then another <span v-if="tasks.length > N"> that displays the text "And ${tasks.length - N} more" as before.

Added a simple unit test, but would be good if someone else tested to confirm the values displayed make sense. I didn't use this feature in Cylc 7, so I'm not 100% sure that it's working OK. 👍

hjoliver · 2019-11-11T07:43:14Z

Nice, thanks @kinow. Looks great, works great!

We'll need sorting by "time" not name (e.g. when looking at succeeded tasks, want the most recently succeeded ones first). But that can be a follow-up. This is great for my demo as-is (I won't tell anyone it's not really "most recent", he heh 😁 )

The Cylc 7 version also does this server side (I had forgotten) but it is probably better in principle that the UI does it (some future UI component might need to list all tasks in a state, not just a few.)

hjoliver · 2019-11-11T22:55:46Z

@oliver-sanders - I'm going to merge this without your approval, as I need it in for demos soon. I believe all your comments have been addressed above, with some things punted to other issues (and/or updated the gscan Issue description). Note two questions on things that might need a new Issue page.

kinow added this to the 0.2 milestone Nov 5, 2019

kinow self-assigned this Nov 5, 2019

kinow mentioned this pull request Nov 5, 2019

Add slots to Tree component #299

Closed

9 tasks

kinow requested review from hjoliver and oliver-sanders November 5, 2019 03:52

oliver-sanders reviewed Nov 6, 2019

View reviewed changes

kinow force-pushed the add-gscan-component branch from 239778b to a1ea837 Compare November 6, 2019 22:23

kinow mentioned this pull request Nov 8, 2019

Query ownership #304

Closed

hjoliver approved these changes Nov 9, 2019

View reviewed changes

kinow force-pushed the add-gscan-component branch from 136e1e3 to 45ed91d Compare November 10, 2019 09:49

kinow commented Nov 10, 2019

View reviewed changes

src/components/cylc/Drawer.vue Outdated Show resolved Hide resolved

kinow force-pushed the add-gscan-component branch from 45ed91d to eb4fff7 Compare November 10, 2019 21:37

kinow commented Nov 10, 2019

View reviewed changes

Bruno P. Kinoshita and others added 20 commits November 11, 2019 19:58

Add GScan Cylc component

ea25589

Initialize GraphQL data on application startup

0802bbe

Replace links by GScan component

2e06c17

Add unit tests

933a295

Add changelog

d464463

Remove node-sass dependency

ca1e5ff

Store workflowTree in a computed variable

6d102ef

Ignore console error when an exception happens creating the workflowTree

815b9df

Do not use user object directly

2c95c1c

Fix toolbar title

72a77f3

Use just the workflowName prop, no need for extra workflowId

3b28514

Adjust unit tests

5b9dba1

Revert removal of the workflows link

3e43f27

Move queries/data-fetch to component

57a9c37

Return workflow icon function

1c8e37b

Remove elevation props from v-btn (fixing borders in ffox)

157b64c

Defensive programming on accessing GraphQL nodes and edges data

96ee7c0

Create nodes and edges only for the current workflow

f64ff21

Manually add names to mock taskproxy data

d437a52

Sort tasks alphabetically within each state, and limit the number of …

fc9f14f

…tasks displayed to 5

kinow force-pushed the add-gscan-component branch from e3575dc to fc9f14f Compare November 11, 2019 06:58

hjoliver merged commit e67f345 into cylc:master Nov 11, 2019

kinow mentioned this pull request Nov 12, 2019

Create Workflow (icon?) component #190

Closed

kinow deleted the add-gscan-component branch March 19, 2020 00:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add gscan component #301

Add gscan component #301

kinow commented Nov 5, 2019

kinow commented Nov 5, 2019 •

edited

Loading

kinow commented Nov 5, 2019

codecov-io commented Nov 5, 2019 •

edited

Loading

kinow commented Nov 5, 2019

kinow commented Nov 5, 2019

oliver-sanders left a comment

oliver-sanders Nov 6, 2019

hjoliver Nov 11, 2019

oliver-sanders Nov 6, 2019

kinow Nov 6, 2019

hjoliver Nov 11, 2019

kinow commented Nov 6, 2019

hjoliver commented Nov 8, 2019

kinow commented Nov 8, 2019

kinow commented Nov 8, 2019 •

edited

Loading

kinow commented Nov 8, 2019

hjoliver commented Nov 9, 2019

hjoliver left a comment

kinow commented Nov 9, 2019

hjoliver commented Nov 10, 2019

kinow commented Nov 10, 2019 •

edited

Loading

hjoliver commented Nov 10, 2019

kinow commented Nov 10, 2019 •

edited

Loading

kinow Nov 10, 2019 •

edited

Loading

kinow Nov 10, 2019 •

edited

Loading

kinow Nov 10, 2019

hjoliver Nov 11, 2019 •

edited

Loading

hjoliver commented Nov 11, 2019

kinow commented Nov 11, 2019

hjoliver commented Nov 11, 2019

kinow commented Nov 11, 2019 •

edited

Loading

hjoliver commented Nov 11, 2019

hjoliver commented Nov 11, 2019

Add gscan component #301

Add gscan component #301

Conversation

kinow commented Nov 5, 2019

kinow commented Nov 5, 2019 • edited Loading

kinow commented Nov 5, 2019

codecov-io commented Nov 5, 2019 • edited Loading

Codecov Report

kinow commented Nov 5, 2019

kinow commented Nov 5, 2019

oliver-sanders left a comment

Choose a reason for hiding this comment

oliver-sanders Nov 6, 2019

Choose a reason for hiding this comment

hjoliver Nov 11, 2019

Choose a reason for hiding this comment

oliver-sanders Nov 6, 2019

Choose a reason for hiding this comment

kinow Nov 6, 2019

Choose a reason for hiding this comment

hjoliver Nov 11, 2019

Choose a reason for hiding this comment

kinow commented Nov 6, 2019

hjoliver commented Nov 8, 2019

kinow commented Nov 8, 2019

kinow commented Nov 8, 2019 • edited Loading

kinow commented Nov 8, 2019

hjoliver commented Nov 9, 2019

hjoliver left a comment

Choose a reason for hiding this comment

kinow commented Nov 9, 2019

hjoliver commented Nov 10, 2019

kinow commented Nov 10, 2019 • edited Loading

hjoliver commented Nov 10, 2019

kinow commented Nov 10, 2019 • edited Loading

kinow Nov 10, 2019 • edited Loading

Choose a reason for hiding this comment

kinow Nov 10, 2019 • edited Loading

Choose a reason for hiding this comment

kinow Nov 10, 2019

Choose a reason for hiding this comment

hjoliver Nov 11, 2019 • edited Loading

Choose a reason for hiding this comment

hjoliver commented Nov 11, 2019

kinow commented Nov 11, 2019

hjoliver commented Nov 11, 2019

kinow commented Nov 11, 2019 • edited Loading

hjoliver commented Nov 11, 2019

hjoliver commented Nov 11, 2019

kinow commented Nov 5, 2019 •

edited

Loading

codecov-io commented Nov 5, 2019 •

edited

Loading

kinow commented Nov 8, 2019 •

edited

Loading

kinow commented Nov 10, 2019 •

edited

Loading

kinow commented Nov 10, 2019 •

edited

Loading

kinow Nov 10, 2019 •

edited

Loading

kinow Nov 10, 2019 •

edited

Loading

hjoliver Nov 11, 2019 •

edited

Loading

kinow commented Nov 11, 2019 •

edited

Loading