protobuf 3.12 optional fields pre-release

[dwsutherland]

These changes close #3589

Held icons no longer linger in WUI/TUI 🎉

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).
• Already covered by existing tests.
• No change log entry required (invisible to users).
• No documentation update required.

[dwsutherland]

All fields set as optional where needed, so if any are set (to any value) that will be read as set by the receiver ...

We can reassess individual field behaviour once/if the protobuf mixing issue (above) is fixed, otherwise I don't see any harm in being intentional/explicit about which fields are set and which to clear (i.e. field.Clear())

[hjoliver]

Should this go in before the delta-PRs? (Does it matter?)

[dwsutherland]

Should this go in before the delta-PRs? (Does it matter?)

Yes, can go in first, if there are conflicts delta-PR to resolve.

[hjoliver]

👍 looks fine, pretty easy review given that the .py file is generated!

[kinow]

Code looks good. I had a go with cylc-uiserver (master) and cylc-flow (master and then this branch). Everything worked. I tried cylc hold five foo.*, but the icon worked the same for both master and this branch? i.e. upon held, the icon got a pause badge. Then cylc release five foo.* every pause badge disappeared and the workflow continued.

No errors in the terminal or JS console, so +1

[dwsutherland]

I tried cylc hold five foo.*, but the icon worked the same for both master and this branch?

Strange, they don't disappear after release on master for me. Did the UI Server re-install with 3.12.0rc1?

[kinow]

Strange, they don't disappear after release on master for me. Did the UI Server re-install with 3.12.0rc1?

Could be my suite that's too simple maybe (five, running jobs with sleep 2). I re-installed the Cylc Flow module only. UI Server was still on master.

[dwsutherland]

Strange, they don't disappear after release on master for me. Did the UI Server re-install with 3.12.0rc1?

Could be my suite that's too simple maybe (five, running jobs with sleep 2). I re-installed the Cylc Flow module only. UI Server was still on master.

It's only the old tasks who's isHeld doesn't change from true.. So if they disappear to quickly, then yeah, we wouldn't notice it.

UI Server master would have to reinstall also, because cylc-flow dependencies get installed too. (right?)

[hjoliver]

I can get "stuck" hold icons in cylc tui on master on a slightly more complex suite. It's hard to know exactly how to reproduce, but a few hold release cycles will result in a few of them.

[dwsutherland]

I can get "stuck" hold icons in cylc tui on master on a slightly more complex suite. It's hard to know exactly how to reproduce, but a few hold release cycles will result in a few of them.

Yes, you should get lingering held icons for cylc tui on master.. But after reinstalling with this branch, they should disappear on release.

[dwsutherland]

Ah I think I know why... I had a workaround in place that set the held and reloaded booleans back to False on delta application:

cylc-flow/cylc/flow/data_store_mgr.py

Lines 142 to 143 in 37e676e

	if not delta.reloaded:
	data[key].reloaded = False

cylc-flow/cylc/flow/data_store_mgr.py

Lines 155 to 156 in 37e676e

	if not element.is_held:
	data[key][element.id].is_held = False

Can get rid of them now..

We'll still have the issue of jobs outside the task pool window retaining isHeld (but that's separate to this)

[oliver-sanders]

Approved in principle.

[hjoliver]

Approved in principle.

Merging in practice 😁