Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
ColemanTom opened this issue Jul 16, 2024 · 13 comments
Open

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

ColemanTom opened this issue Jul 16, 2024 · 13 comments
Labels
bug Something isn't working small

Comments

@ColemanTom
Copy link

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.

@ColemanTom ColemanTom added the bug Something isn't working label Jul 16, 2024
@hjoliver
Copy link
Member

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
Copy link
Member

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

@ColemanTom
Copy link
Author

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
Copy link
Member

hjoliver commented Jul 17, 2024

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 oliver-sanders transferred this issue from cylc/cylc-ui Jul 17, 2024
@oliver-sanders
Copy link
Member

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

@wxtim wxtim self-assigned this Aug 1, 2024
@wxtim
Copy link
Member

wxtim commented Aug 1, 2024

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
Copy link
Member

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 🤦.

@oliver-sanders oliver-sanders transferred this issue from cylc/cylc-flow Aug 1, 2024
@MetRonnie
Copy link
Member

@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
Copy link
Member

oliver-sanders commented Aug 9, 2024

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
Copy link
Member

wxtim commented Aug 23, 2024

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
Copy link
Member

Fix requires a UI code change, see #1876 (comment)

@wxtim
Copy link
Member

wxtim commented Aug 23, 2024

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

@oliver-sanders
Copy link
Member

Just UI code change.

@wxtim wxtim removed their assignment Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working small
Projects
None yet
Development

No branches or pull requests

5 participants