uis subscription data sync info (UIS-597) - #6113
Conversation
50e7c94 to
2e63d5e
Compare
0d033e1 to
1dee25b
Compare
b86c3e4 to
542ccc6
Compare
|
Reminder (largely for myself): This PR is pending an upstream implementation in cylc-uiserver. Drafting for now as this code may change in relation to upstream requirements. |
|
(unassigning this to re-focus my review queue, feel free to re-assign when undrafted) |
542ccc6 to
ac312a7
Compare
ac312a7 to
3369d88
Compare
3369d88 to
e4a2eed
Compare
|
Don't think the lint failure is relevant:
|
685725a to
bfade96
Compare
bfade96 to
8666039
Compare
8666039 to
a3cad8f
Compare
a3cad8f to
82696b0
Compare
82696b0 to
eb1c6ed
Compare
0a3398b to
af67840
Compare
af67840 to
8c0552f
Compare
oliver-sanders
left a comment
There was a problem hiding this comment.
As I understand it:
- When a subscription is started, or a query is issued, the timer field is set.
- Once the timeout has elapsed, the subscription will be recalculated.
- (When a subscription is stopped, we should also set the timer, but I'm not sure we currently are?)
Just wondering if it might be easier to add hook methods, e.g, query_issues, subscription_started, subscription_stopped to avoid having to set and track these timeouts, e.g:
def query_issued(self, w_id):
self.loop.call_later(TIMEOUT, self.recalculate_subscription, w_id)
def subscription_stopped(self, w_id):
self.loop.call_later(TIMEOUT, self.recalculate_subscription, w_id)| data_store[w_id] | ||
| for w_id in iter_uniq(w_ids) | ||
| if w_id in data_store | ||
| ] |
There was a problem hiding this comment.
This is the same as the existing function, just with the addition of the await self.data_store_mgr.set_query_sync_levels(set(w_ids)) bit.
Consequently, we have to duplicate the if is_sub: ...; else logic to toggle between these two implementations for all uses below.
I think we can simplify things by merging the two implementations like so:
async def get_flow_data_from_ids(self, data_store, native_ids, is_sub):
w_ids = {Tokens(w_id).workflow_id for w_id in native_ids}
if is_sub:
await self.data_store_mgr.set_query_sync_levels(w_ids)
return [data_store[w_id] for w_id in w_ids if w_id in data_store]Then pass trough a bool flag to toggle:
flow_data = get_flow_data_from_ids(
self.data_store_mgr.data,
nat_ids,
bool(args['delta_store'])
)There was a problem hiding this comment.
Will have to check if it needs to be order preserving (i.e. for tests), but yeah maybe a list would work the same for query sync level setting.
There was a problem hiding this comment.
Although there may be some simplification not thought of, both calls also pass in a different store: self.delta_store[args['sub_id']] and self.data_store_mgr.data according to args['delta_store']..
And although args['delta_store'] implies is_sub, some subs resolve from the main store self.data_store_mgr.data .. So bool(args['delta_store']) can be False while is_sub is True.
(I did attempt simplification for a while.. but got stuck on this compication)
8c0552f to
98b95e7
Compare
Off the top of my head, there's no timer on subscriptions (stays up for as long as the subscription is active) The query data types are added to the |
|
Back compatibility in: |
a4f9e31 to
bc916d6
Compare
sibling of cylc/cylc-uiserver#597
Have included testing some of the changes, however the parts not tested are used by cylc-uiserver not cylc-flow.
This end is to:
Check List
CONTRIBUTING.mdand added my name as a Code Contributor.setup.cfg(andconda-environment.ymlif present).CHANGES.mdentry included if this is a change that can affect users?.?.xbranch.