cylc-uiserver icon indicating copy to clipboard operation
cylc-uiserver copied to clipboard

Sync minimal flow data

Open dwsutherland opened this issue 1 year ago • 10 comments

addresses https://github.com/cylc/cylc-uiserver/issues/585 addresses https://github.com/cylc/cylc-uiserver/issues/568

This change aims to:

  • [x] interrogate the graphql subscription for data requirements.
  • [x] Sync only the data required.
  • [x] Reduce the protobuf subscription topics and all data dump size.
  • [x] Handle data requirements for queries.

This first attempt is functional, reads the fragments requested by the UI with {'AddedDelta', 'WorkflowData', 'UpdatedDelta'} being the workflow summary information of the dashboard subscription.

The UI doesn't seem to reduce the subscription if you go from tree view to dashboard, however, closing the UI will reduce it back down.

One thing we need to be able to handle is general queries hitting the UIS, as we'll want CLI via UIS in at some point and to be able to handle random queries.. For this I think we can use that timer concept, keeping a full sync of workflows queried for some amount of time.. Done

Check List

  • [x] I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • [x] Contains logically grouped changes (else tidy your branch by rebase).
  • [x] Does not contain off-topic changes (use other PRs for other changes).
  • [x] Applied any dependency changes to both setup.cfg (and conda-environment.yml if present).
  • [ ] Tests are included (or explain why tests are not needed).
  • [ ] CHANGES.md entry included if this is a change that can affect users
  • [x] Cylc-Doc pull request opened if required at cylc/cylc-doc/pull/XXXX.
  • [x] If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

dwsutherland avatar May 29 '24 03:05 dwsutherland

more updates inc..

dwsutherland avatar May 29 '24 09:05 dwsutherland

Ok I've added more features and taken some ideas from #568 .. Turns out you can subscribe/unsubscribe from topics without restarting the connection: https://github.com/cylc/cylc-uiserver/issues/568

If you print the topic near the top of the data_store_mgr.py _update_workflow_data method, you can see it changing when you open and close the graph view: image

It's even more noticeable with multiple workflows running, and switching between them in the UI. image

dwsutherland avatar May 30 '24 07:05 dwsutherland

Not sure the reason for this mypy failure: image

new_data[field.name].CopyFrom(value) has always been there... and passes locally

Also, I get a lot of this:

ERROR cylc/uiserver/tests/test_workflows_mgr_data_store.py::test_data_store_changes[active-active-actions8] - AttributeError: 'CylcUIServer' object has no attribute 'profiler'. Did you mean: 'profile'?

ERROR cylc/uiserver/utils.py::uiserver.utils.fmt_call - AttributeError: 'CylcUIServer' object has no attribute 'profiler'. Did you mean: 'profile'?

from pytest locally.. I assume I'm missing some sort of profiler

dwsutherland avatar May 30 '24 08:05 dwsutherland

provided we don't need to change much of the cylc-flow code

It can go in now, and not break anything.. So if we can at least do that for 8.3.0, then we can move forward with this later.

I don't think that we can reliably determine the topic(s) we need to subscribe to by looking at the GraphQL fragments

We can, and it works, but yes not reliable (if UI graphql changes, it could break it).. if we could pick up the fragments from the associated UI code then maybe.. For any subscription that doesn't contain those fragments (i.e. a bespoke subscription) it will default to max. Parsing for every field requiring resolvers into sets could be another way.. Should only need to happen at the start of a new subscription, so we can probably get away with something more intensive then.

dwsutherland avatar May 30 '24 11:05 dwsutherland

Have a solution for queries, hopefully post it up tomorrow..

dwsutherland avatar Jun 10 '24 12:06 dwsutherland

Query sync level change and expire implemented, and functional: image

The expiry is set to 60sec, with a periodic callback every 30sec to check and demote. (~30sec expiry in the above example)

The expiry will take precedence over subscription start/stops, so the level will remain. Otherwise the levels promote/demote immediately in accordance with subscription start/stop.

dwsutherland avatar Jun 12 '24 01:06 dwsutherland

provided we don't need to change much of the cylc-flow code

It can go in now, and not break anything.. So if we can at least do that for 8.3.0, then we can move forward with this later.

I was wrong on this, I think the current UIS will break with the cylc-flow end in first (because the data-store manager on the UIS doesn't contain the methods that will be called by the resolvers)

So they both need to go in .. which is a shame .. because I assume the next 8.x release will be some way off..

dwsutherland avatar Jun 18 '24 00:06 dwsutherland

I think I may have found an easier way to hook into the query/(subscriptions) before execution .. Actually, at other points of the execution too: https://github.com/graphql-python/graphene-tornado/blob/master/graphene_tornado/graphql_extension.py

So, probably a good thing it doesn't go in yet

dwsutherland avatar Jun 18 '24 05:06 dwsutherland

because I assume the next 8.x release will be some way off..

8.3.0 was a monster, we're hoping to resume a more regular release cycle once this is out the way, 8.4.0 could be ~1 month away.

However, back compat is important, otherwise the GUI won't be able to connect to older workflows, so a cylc-uiserver side approach is definitely preferable. Note, if we do need to cut off back-compat, the way we do this is to increment the API version number in cylc-flow (currently 5). There is an API number filter in the scan mechanism in cylc-uiserver which will filter out older or newer workflows avoiding runtime problems.

oliver-sanders avatar Jun 18 '24 08:06 oliver-sanders

Reminder (largely for myself): This work requires an interface for tracking subscriptions so we can tell what things users are actively subscribed to. This should work by parsing the workflows GraphQL argument or similar (we can't rely on subscription query format, variable name conventions, etc).

We should be good to resume this work post the graphql-core v3 changes.

oliver-sanders avatar Aug 18 '25 14:08 oliver-sanders