Include severity in delta for task message

[MetRonnie]

Closes #5713

This will allow the UI to know the severity level of task messages, which will be used to colour the message chips in the tree view - https://github.com/cylc/cylc-ui/pull/1436

By not changing the GraphQL schema this is the simplest way to address #5713

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] No dependency changes
• [x] No tests are included
• [x] No changelog entry as minor
• [x] No docs needed
• [x] If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

[oliver-sanders]

Wouldn't it be better to add this to the schema?

[MetRonnie]

How would you propose adding it to the schema?

[oliver-sanders]

Looks like we didn't leave room for this:

job {
  id
  messages {
    severity
    message
  }
}

[MetRonnie]

Yeah that was my thought

[oliver-sanders]

Ok, we do have a pathway for handling schema changes, it's just something we haven't had to do yet.

Note, we don't need to put in back-compat for GraphQL because the communication between the UI and UIS uses the UIS schema version not the scheduler version. We only need to provide back-compat for the resolver to allow the UIS to provide responses in the required format for older schedulers where the required field is not present in the store.

1. Add a new field to the data store to hold the severity.

   diff --git a/cylc/flow/data_messages.proto b/cylc/flow/data_messages.proto
   index ced0cad7b..cb0bbff41 100644
   --- a/cylc/flow/data_messages.proto
   +++ b/cylc/flow/data_messages.proto
   @@ -128,6 +128,10 @@ message PbRuntime {
        optional string outputs = 17;
    }
   
   +message PbMessage {
   +    optional string message = 1;
   +    optional string severity = 1;
   +}
   
    // Nodes
    message PbJob {
   @@ -147,7 +151,7 @@ message PbJob {
        repeated string extra_logs = 29;
        optional string name = 30; /* filter item */
        optional string cycle_point = 31; /* filter item */
   -    repeated string messages = 32;
   +    repeated PbMessage messages = 32;
        optional PbRuntime runtime = 33;
    }
   

   Note, I don't know whether protobuf validates messages at the receiver end, if so we may need to use a new field to avoid conflict issues.

2. Change the GraphQL schema:

   job {
     id
     messages {
       message
       severity
     }
   }
   

3. Add back-compat in the UIS to fill in this field when not present in the store:

   class UISJob:
     messages(severity=String, message=String, resolver=get_messages)
   
   def get_messages(...):
       if node.messages and len(node.messages[0]) < 1:
           # BACK COMPAT: message without severity information
           # REMOVE AT: 8.5.0
           return [['INFO', message] for message in node.messages]
       return node.messages
   

4. Update the UI.

5. In a later version, remove the back-compat and increase the minimum cylc-flow version at the UIS:

   diff --git a/cylc/uiserver/workflows_mgr.py b/cylc/uiserver/workflows_mgr.py
   index e441c38..df32c61 100644
   --- a/cylc/uiserver/workflows_mgr.py
   +++ b/cylc/uiserver/workflows_mgr.py
   @@ -40,6 +40,7 @@ from cylc.flow.network.client import WorkflowRuntimeClient
    from cylc.flow.network.scan import (
        api_version,
        contact_info,
   +    cylc_version,
        is_active,
        scan,
        validate_contact_info
   @@ -145,6 +146,7 @@ class WorkflowsManager:  # noqa: SIM119
                | validate_contact_info
                # only flows which are using the same api version
                | api_version(f'=={API}')
   +            | cylc_version(f'>= 8.5.0')
             )
            # queue for requesting new scans, valid queued values are:
   

[oliver-sanders]

It's a bit more faff but it gets the end result nicer. Note if we don't do this, we will need to be able to reliably strip the message severity in the UI. Currently messages are compared to configured task outputs to differentiate between the two

[MetRonnie]

Note if we don't do this, we will need to be able to reliably strip the message severity in the UI. Currently messages are compared to configured task outputs to differentiate between the two

What's the downside to this approach? The only thing I can think of is if a user plays a workflow with a new version of cylc-flow but an old version of the UI, then they will see chips saying INFO:started etc?

(btw I suppose this should be on 8.3.0 milestone, changing now)

[hjoliver]

Ok, we do have a pathway for handling schema changes, it's just something we haven't had to do yet.

Can you copy-and-paste that procedure somewhere for future reference?

[dwsutherland]

Note, I don't know whether protobuf validates messages at the receiver end, if so we may need to use a new field to avoid conflict issues.

I think it will try to validate that PbMessage and/or error reading it in, so scheduler and UIS would have to have the same version.. You can make a new field entry, but then I suppose you'd have to have a different name...

Alternatively you could create a new field optional string message_severity = 34; which would solve the problem too.

[oliver-sanders]

Ok, it looks like the upgrade option would be:

• Add a new protobuf field, and remove the old one at a future point.
• Back support the new field with severity = data.get('new-field', 'INFO').

Otherwise, we dump both bits of info into the same field:

• Application will need to split the string to get the severity and message.
• The UIS will need to join the severity and message for offline data.