search

cylc / cylc-uiserver

A Jupyter Server extension that serves the cylc-ui web application for monitoring and controlling Cylc workflows.
https://cylc.org
GNU General Public License v3.0
15 stars 18 forks source link

graphql: delta fixes #221

Open oliver-sanders opened 3 years ago

oliver-sanders commented 3 years ago

Address remaining delta issues:

See https://github.com/cylc/cylc-uiserver/pull/212#issuecomment-841107713

(really a cylc-flow issue?)

dwsutherland commented 3 years ago

I don't think it should be updated, the UI should rebuild it's store on:

Main reason being; there may be no way to reconcile the update with the old UI store (i.e. the workflow changed it's structure, so historical nodes may never get a pruned delta).. It's the same reason I don't update the scheduler store on reload... We can still show orphaned tasks (we don't at present, issue up somewhere), and change it's family hierarchy (by throwing it under root), but it's best start fresh on these rare occurrences.

oliver-sanders commented 3 years ago

Ok, I was unaware that these extra added deltas were supposed to be signals to the UI to rebuild its store (I'm not sure whether the UI is aware of that either?). It wasn't obvious to me that added(status:running) meant "delete everything and rebuild from scratch".

If we have to wipe the entire UI data store entry then the UI will have to clear all DOM elements relating to those entries meaning that on (re)start/reload the workspace would go blank before reloading and the user would loose all of their data, selection and position within the app (but the tabs would be preserved at least). Which wouldn't be great, if there's a way of resolving this outside of the UI code it's worth investigating.

I'm not convinced that we should ever need to rebuild the UI store (other than in the event of validation failure at the UI end)? In the event of "start" and "reload" the UIS should be able to tell what's changed and send that in an updated delta? Or at least tell what items have changed and send a "pruned" and "added" delta for each? Can the UIS tell what's changed something like this?:

before = set(data[TaskProxies])
apply_delta()
after = set(data[TaskProxies])
pruned = before - after
added = after - before
updated = before & after
yield make_delta(TaskProxy, pruned, added, updated)

If we do need to tell the UI to rebuild the store could we send an explicit signal to the UI telling it what to clear rather than leaving it to interpret the signal itself. Perhaps something along these lines:

# restart
pruned(taskProxies:*, jobs:*, tasks:*)
updated(status:running)

One problem that we are going need to address soon(ish) is that the GraphQL deltas will need to be used to provide both offline and live data. The user might be looking at cycles outside of the n=x window, viewing the static graph or job log files. In this case we wouldn't want the UI to delete all of that data, this would be basically equivalent to refreshing the browser tab.

dwsutherland commented 3 years ago

Not saying it's not doable, it's definitely idealistic... I just think it's not worth our effort right now:

If we have to wipe the entire UI data store entry then the UI will have to clear all DOM elements relating to those entries meaning that on (re)start/reload the workspace would go blank before reloading and the user would loose all of their data, selection and position within the app (but the tabs would be preserved at least). Which wouldn't be great, if there's a way of resolving this outside of the UI code it's worth investigating.

Surely the UI can rebuild then do a switch before deleting the old one.

One problem that we are going need to address soon(ish) is that the GraphQL deltas will need to be used to provide both offline and live data. The user might be looking at cycles outside of the n=x window, viewing the static graph or job log files. In this case we wouldn't want the UI to delete all of that data, this would be basically equivalent to refreshing the browser tab.

Perhaps that can also be part of the burst, but history may not make sense after a reload.. i.e. if your graph went from hundreds of tasks to a dozen or something with completely different dependencies... Usually it's the user who's doing the reloads anyway.

The main benefit I see from sending deltas between discontinuities (reload ..etc) is; being able to present a diff to the user at the UI.

oliver-sanders commented 8 months ago

Note, the UI currently works around this issue by not differentiating between added and updated deltas and creating any intermediate nodes which appear in the delta but are not (yet) present in the store.

This behaviour was introduced in https://github.com/cylc/cylc-ui/pull/1108, it resolved the "updated delta before added delta" error messages that were previously appearing.