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

internals: use of TASK_OUTPUTs and TASK_EVENTs in task_event_mgr #6002

Open
hjoliver opened this issue Feb 28, 2024 · 3 comments
Open

internals: use of TASK_OUTPUTs and TASK_EVENTs in task_event_mgr #6002

hjoliver opened this issue Feb 28, 2024 · 3 comments
Labels
code refactor Large code refactors small
Milestone

Comments

@hjoliver
Copy link
Member

Noticed while working on #5658

Straighten out the use of TASK_OUTPUT_blah vs TASK_EVENT_blah in the event manager module.

It works fine as-is, because the standard OUTPUT and EVENT names are the same, but we should still use the correct constants for clarity.

@hjoliver hjoliver added small code refactor Large code refactors labels Feb 28, 2024
@hjoliver hjoliver added this to the cylc-8.x milestone Feb 28, 2024
@MetRonnie
Copy link
Member

What is the problem with how it is currently?

@oliver-sanders
Copy link
Member

oliver-sanders commented Mar 12, 2024

There are four related things:

  • Outputs - sometimes used to mean trigger:message pairs, but sometimes used to refer to triggers or messages.
  • Triggers - the things you can use in the graph or satisfy with cylc set. These are often referred to as "labels".
  • Messages - the strings you can send through cylc message.
  • Events - kinda like messages that come from within the system like submission failed or retry, but confusingly, also started, succeeded and failed which are also task outputs.

Whilst working on #5658 it was discovered that triggers/messages were sometimes mislabelled as their opposite and outputs was sometimes used ambiguously to refer to one or the other. No functional problems, it all works fine, just confusing when you need to work on this code.

Cheat Sheet:

  • DB - messages
  • Data Store - messages & triggers
  • TaskOutputs - messages & triggers
  • cylc set - triggers
  • cylc message - messages
  • cylc trigger - trick question
  • Scheduler.message_queue - messages & events
  • TaskEventsManager.process_message - messages & events

@oliver-sanders
Copy link
Member

oliver-sanders commented Mar 12, 2024

I've been diving into this code over the past few days whilst working on the optional outputs and struggling with this a bit (completion expressions need triggers, most of the rest of the code needs messages, trying to avoid storing surplus maps whilst keeping it efficient). I'm coming to the conclusion that messages should be mapped onto triggers in the task events code, from there on it would be simpler to stick to triggers.

Triggers are the more fundamental and user facing concept. They are used in the graph, the completion expression, cylc show, universal ID, etc. Messages can be used to provide supplementary detail and may be patterns, great for sending supplementary information back to the scheduler (not that anyone does sadly, I've checked!), but not so good for internal interfaces where it's not clear whether we're dealing with raw messages or matching message patterns. Note raw messages may not be unique and the mapping between raw messages and patterns may be broken by reload/restart.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code refactor Large code refactors small
Projects
None yet
Development

No branches or pull requests

3 participants