cylc / cylc-ui

Web app for monitoring and controlling Cylc workflows
https://cylc.github.io
GNU General Public License v3.0
37 stars 27 forks source link

Create views off the data store (instead of only using initial-data-burst) #803

Closed kinow closed 1 year ago

kinow commented 2 years ago

Describe exactly what you would like to see in an upcoming release

At the moment the views like Tree and Table view are created using Vuex store properties. These properties are only ever initialized from GraphQL initial-data-burst messages.

Note that the data in the Vuex store that we use is derived from GraphQL data. e.g. the Tree View doesn't use GraphQL data directly. The GraphQL endpoint doesn't provide hierarchical data. Instead, there is some code that creates a hierarchical data structure with pointers to the GraphQL original data.

Additional context

The challenge here, then, is to think in a way to create the derived data structure in an intelligent manner.

We could identify we already have a compatible subscription (i.e. same query used in 2 views), and then just call the code that creates the derived data structure.

Or we could find a way to tie-together the original GraphQL data & the derived data, so that components/views only use the derived data directly (as they are doing now).

The last option is to always refresh a subscription.

NOTE: we identified this issue in #631 where we added a Table View, but already had a Tree View; the two queries are pretty much the same, so merging them results in the same query. See the Table View PR's & issue for more.

Pull requests welcome!

oliver-sanders commented 2 years ago

@kinow Any ideas on how we should move forward with this one?

Some extra context: https://github.com/cylc/cylc-ui/pull/672#issuecomment-898034698

I think we should be able to drive the views from the data store (either the flat or hierarchical structure as desired by the view) rather than from the subscription. Reactivity would come from the data structures in the store rather than from the deltas. We might have to add some filtering in the view to enable this?

As a vague outline something along the lines of this should be possible?:

The Store:

store = [
    {
        'type': workflow,
        'id': 'five',
        taskProxies: { ... },
        ...
    },
]

The View Template:

<template>
    <ul
        v-if="store.workflow_id"
        v-for="taskProxy in store.workflow_id.taskProxies"
    >
        <li>{{taskProxy.id}}</li>
    </ul>
</template>

The View Subscription:

subscription {
    workflows(ids=["five"]) {
        id
        taskProxies {
            id
        }
    }
}
kinow commented 2 years ago

I think we should be able to drive the views from the data store

Agreed

Reactivity would come from the data structures in the store rather than from the deltas.

That would be ideal.

I think the current problem is more what triggers the reactivity. In Vue we would have:

One issue is that we don't always have the reactivity. With Vue I think you always have the reactivity (unless you combine in a computed property another flag that indicates whether the computed is eval'ed or not).

So in the Workflow view, we have 0 or more subscriptions. When we have 0 subscriptions (i.e. no widgets) we have nothing in the data store.

When we have 1 subscription (e.g. workflow GraphQL query, and 1 widget of type tree) we know now that we want to update the raw data, and also create reactively the data structure for the tree.

When we have 1 subscription (e.g. workflow GraphQL query, and 1 widget of type tree, and 1 widget of type table) we know now that we want to update the raw data, and both tree and table data structures.

So we have an occasionally reactive dependency between the raw data, and the table and tree data structure. We could use booleans in the store or in some component that are enabled when you have a tree or table widget, and that toggle the reactivity, i.e.

data () {
  return {
    graphqlData: {...}, // fed from the data from a GraphQL subscription
    hasTreeWidgets: false,
    hasTableWidgets: false
  }
},
computed: {
  treeData () {
    if (this.hasTreeWidgets && this.graphqlData) {
       return [node, node, ...]
    }
    return []
  }
}

One possible simple fix could be to keep things as they are, do not merge the query, but add the callbacks for the new widget. That would enable the occasional-reactivity.

oliver-sanders commented 2 years ago

Here's a slightly clearer explanation of what I'm describing:

We would need some extra housekeeping logic to remove data from the store when we are no longer looking at it. We could do this crudely by removing tasks, taskProxies, etc when there are no longer any active subscriptions to them (i.e. when the user navigates away).

One issue is that we don't always have the reactivity

Ooh, where do we not have reactivity?

and also create reactively the data structure for the tree. we know now that we want to update the raw data, and both tree and table data structures. So we have an occasionally reactive dependency between the raw data, and the table and tree data structure

I think the easy solution is to maintain the flat and tree structures in the store all the time. We can always optimise this later.

(with the current structure I think we will need a graph structure in the store to hold the edge relationships, although we could move that into the task/family proxy sections of the schema to avoid this?)

One possible simple fix could be to keep things as they are, do not merge the query, but add the callbacks for the new widget

I don't think we need to separate the subscriptions, we can blindly update the flat and tree stores with whatever data comes in. The views may have to filter the store data to account for this.

We can always optimise this later to avoid updating things we don't need to.

hjoliver commented 2 years ago

One issue is that we don't always have the reactivity

Ooh, where do we not have reactivity?

?

(Pulling out an important question from the above)

kinow commented 2 years ago

One issue is that we don't always have the reactivity.

@hjoliver , @oliver-sanders , sorry, my example was just bad and what I said is wrong :grimacing: We always have the reactivity, what we don't always have is the data being updated (in a certain way it looks like reactivity, because we have variables that depend on each other, but are only updated when a subscription exists, and updated by callbacks, not by Vue watchers/observers/etc). Even though the variable is reactive, it's just empty.

The tree view uses

In this case, where you are looking at the tree view alone, everything works fine, the callbacks are updating both data structures, everything is reactive.

In the workflow view, you have no data. No subscription. When you add a widget with a view for the first time, then at this point the Workflow view is responsible for starting the subscription, which updates the data.

A tree view added in the workflow view is still using reactive data, but the real issue is that the data for the other view added such as table view was not updated by the workflow view (because it never got the initial data burst to initialize it).

I spoke with @AaronDCole yesterday. I think if we modify the WorkflowService function recompute to merge callbacks regardless of whether a query was merged or not and call the callback if the lookup is not empty, that could solve this issue. Since that would populate the (reactive) data for the table view.

Sorry the confusion!

Here's a slightly clearer explanation of what I'm describing:

@oliver-sanders that's very close to what we have I think. All the bullet points you pointed are currently implemented I think.

The backend updates the flat and tree stores for every delta that comes in.

This is what sometimes I call reactivity, and why I said we have occasional reactivity. Normally you have two variables that depend on each other. Like treeData depends (is computed/getter/watcher/etc) on flatData. Here what we have is a reactivity that exists only when certain subscription+callbacks are subscribed.

We would need some extra housekeeping logic to remove data from the store when we are no longer looking at it.

A view has callbacks that contain code to clear/teardown the data too. They are responsbile for removing data from the store. We never leaver data behind (like the marines :laughing: ). Or at least we try never leave any data behind.

hjoliver commented 2 years ago

Closed by #672 ??

oliver-sanders commented 1 year ago

Closed by https://github.com/cylc/cylc-ui/pull/672 ??

Nope, we are still running the tree and table views off of local data stores powered by local callbacks. Note they are referencing the data in the global data store to avoid duplication. I have taken this one on as part of the graph view work.