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

Broadcast with no namespace does not default to 'root' via the WUI. #1876

Open ColemanTom opened 3 months ago

ColemanTom commented 3 months ago

Description

Using UIServer 2.3.0 as that is what we have installed.

From CLI I am doing the following broadcast to skip a cycle (or namespace): cylc broadcast -p 20240716T0000Z -s 'platform=localhost' -s 'script=true' -s 'pre-script=' -s 'post-script=' -s 'err-script=' -s 'exit-script=' TideNational_dummy. Doing this successfully modifies all tasks to those settings, so everything runs through fast (assuming outputs are not needed of course).

When I try similar from the web UI, the settings do not take and the job continues to run as they normally would.

Reproducible Example

  1. Launch a workflow
  2. Click on the cycle point and see more and then the pencil next to the Broadcast option
  3. Cycle points - the current cycle
  4. Mode - leave as set
  5. Namespaces - leave blank
  6. Settings - add things like
    • platform=localhost
    • script=true
    • pre-script=
    • e.g.

image

From the log file, this is what I see

2024-07-16T04:07:53Z INFO - Command "broadcast" received.
    broadcast(cycle_points=['20240716T0000Z'], mode=put_broadcast, namespaces=['root'], settings=[{'platform': 'localhost'}, {'script': 'true'}, {'pre-script': ''}, {'post-script': ''}, {'err-script': ''}, {'exit-script': ''}])
2024-07-16T04:07:53Z INFO - Broadcast set:
    + [20240716T0000Z/root] platform=localhost
    + [20240716T0000Z/root] script=true
    + [20240716T0000Z/root] pre-script=
    + [20240716T0000Z/root] post-script=
    + [20240716T0000Z/root] err-script=
    + [20240716T0000Z/root] exit-script=

...

2024-07-16T04:09:54Z INFO - Command "broadcast" received.
    broadcast(cycle_points=['20240717T0000Z'], mode=put_broadcast, namespaces=[None], settings=[{'script': 'true'}, {'platform': 'localhost'}])
2024-07-16T04:09:54Z INFO -

The first broadcast was via CLI, the second via the UI. The Broadcast set line doesn't appear, its just an empty "INFO".

If I add in root to the namespace section, the above command does work, but, my interpretation is it should not be needed as the default should be root based on the CLI. If it is intentional that the default is None, then please state that in th tooltips.

Expected Behaviour

Broadcast via CLI and WUI should lead to the same results when no namespace is specified.

hjoliver commented 3 months ago

From CLI I am doing the following broadcast to skip a cycle (or namespace):

Interesting! You've just rolled your own "skip mode" - coming in 8.4. https://cylc.github.io/cylc-admin/proposal-skip-mode.html

hjoliver commented 3 months ago

Bug confirmed. From the GUI, namespaces defaults to [None]; from the CLI it's [root].

ColemanTom commented 3 months ago

From CLI I am doing the following broadcast to skip a cycle (or namespace):

Interesting! You've just rolled your own "skip mode" - coming in 8.4. https://cylc.github.io/cylc-admin/proposal-skip-mode.html

Sort of. It's a "we have skip mode at home" imitation. It doesn't clean out the environment section (I don't think you can do [environment]= but I have not tried), stops generation of message triggers which may or may not matter, and corrupts the runtime history as suddenly tasks have taken a second to run instead of their normal time - maybe not a problem if it happens infrequently, but if its a regular occurance then it is annoying.

hjoliver commented 3 months ago

Yeah the official skip mode will be recorded as such in the DB, and has a few bells and whistles, but the concept is more or less the same.

oliver-sanders commented 3 months ago

To resolve, change the default value in the GraphQL schema.

wxtim commented 2 months ago

To resolve, change the default value in the GraphQL schema.

I don't think it's that simple - as far as I can see the default value is already root:

        namespaces = graphene.List(
            NamespaceName,
            description='List of namespaces (tasks or families) to target.',
            default_value=['root']
        )
oliver-sanders commented 2 months ago

Ach, got it.

If you click on a task in order to open the "broadcast" form, then the "namespaces" field will be filled in correctly.

However, if you click on a workflow in order to open the "broadcast" form, then the "namespaces" field will be left blank.

Unfortunately, this is AOTF doing its job correctly. The UI understands that the "namespaces" field can be determined from the context, but if you haven't selected any namespaces in the context, it has nothing to fill in. Unfortunately, to resolve this, I think we would need special logic in AOTF to handle this one case :facepalm:.

MetRonnie commented 2 months ago

@ColemanTom It looks like Edit Runtime is not affected by this and should do what you want for the root family at a particular cycle point

oliver-sanders commented 2 months ago

A quick half-fix might be to adjust the schema (cylc-flow) such that it requires at least one entry to be specified for "namespaces". I think the NON_NULL type should be able to achieve this.

This way it wouldn't let you fire off a command that doesn't target anything.

wxtim commented 2 months ago

A quick half-fix might be to adjust the schema (cylc-flow) such that it requires at least one entry to be specified for "namespaces". I think the NON_NULL type should be able to achieve this.

This way it wouldn't let you fire off a command that doesn't target anything.

Can you check whether this is already done? If I'm reading schema.py it looks like this is already is set, but not working in the desired manner...

https://github.com/cylc/cylc-flow/blob/700ca8465fdf96dfa33d2d8eb819835236cea79d/cylc/flow/network/schema.py#L1704

oliver-sanders commented 2 months ago

Fix requires a UI code change, see https://github.com/cylc/cylc-ui/issues/1876#issuecomment-2262645483

wxtim commented 2 months ago

Having an AOTF education - fix required UI code change or UI and schema code change?

oliver-sanders commented 2 months ago

Just UI code change.