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

workflow-state: handle submitted as a message/trigger not status #6157

Closed
oliver-sanders opened this issue Jun 18, 2024 · 2 comments · Fixed by #6175
Closed

workflow-state: handle submitted as a message/trigger not status #6157

oliver-sanders opened this issue Jun 18, 2024 · 2 comments · Fixed by #6175
Assignees
Labels
bug Something is wrong :( small
Milestone

Comments

@oliver-sanders
Copy link
Member

The workflow-state command defaults to polling task status rather than task trigger.

The CLI upgrade advise tells you to upgrade an existing message poller into a status poller:

$ cylc workflow-state generic --point 20191209T1200Z --task bar --message=submitted
WARNING - --task, --status, --message, --output, --point, --task-point are deprecated. Please use an ID:
    generic//20191209T1200Z/bar:submitted

This is inconsistent with the xtrigger upgrade advice which tells you to use is_message=True:

        workflow_state(message=submitted, point=1234, task=foo, workflow=ab) -->
    workflow_state(workflow_task_id=ab//1234/foo:submitted, is_message=True)

The CLI advice becomes a problem in the case of the submitted output. Because submitted is a status, it defaults to status polling. However, the submitted status is transitory so this test is not safe.

Suggest:

  • Changing the CLI upgrade advice to match the xtrigger advice.
  • Raising a warning if workflow status polling is configured for non-final statuses.
  • Document trigger polling as the preferred approach.
@oliver-sanders oliver-sanders added bug Something is wrong :( small labels Jun 18, 2024
@oliver-sanders oliver-sanders added this to the 8.3.1 milestone Jun 18, 2024
@oliver-sanders oliver-sanders changed the title workflow-state: handle "submitted" as workflow-state: handle "submitted" as a message or trigger not a statuds Jun 18, 2024
@oliver-sanders oliver-sanders changed the title workflow-state: handle "submitted" as a message or trigger not a statuds workflow-state: handle "submitted" as a message or trigger not a status Jun 18, 2024
@oliver-sanders oliver-sanders changed the title workflow-state: handle "submitted" as a message or trigger not a status workflow-state: handle submitted as a message/trigger not status Jun 18, 2024
@hjoliver
Copy link
Member

hjoliver commented Jun 18, 2024

I commented on this in the recent workflow-state overhaul PR, here:

#5809 (comment)

Initially I did automatically convert transient statuses to outputs, but backed out of that for the following reasons:

  • the command and xtrigger names (-state) suggest status should be the default (outputs were latecomers to this functionality)
  • continuity with all prior usage
  • it might be surprising if a user asks for :running (status) and gets :started (output)
  • it seems possible that the user only wants to know the instantaneous status

Maybe that was the wrong call, but @MetRonnie seemed satisfied at the time.

@hjoliver
Copy link
Member

The CLI upgrade advise tells you to upgrade an existing message poller into a status poller:

Definitely not deliberate.

Suggest:

Agreed on all counts.

oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this issue Jun 27, 2024
* Do not allow users to poll for transient statuses.
* Reject invalid task states.
* Reject polling waiting and preparing tasks (not reliably pollable).
* Closes cylc#6157
oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this issue Jun 27, 2024
* Do not allow users to poll for transient statuses.
* Reject invalid task states.
* Reject polling waiting and preparing tasks (not reliably pollable).
* Closes cylc#6157
@oliver-sanders oliver-sanders self-assigned this Jun 27, 2024
oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this issue Jun 28, 2024
* Do not allow users to poll for transient statuses.
* Reject invalid task states.
* Reject polling waiting and preparing tasks (not reliably pollable).
* Closes cylc#6157
@oliver-sanders oliver-sanders modified the milestones: 8.3.1, 8.3.2 Jul 3, 2024
@MetRonnie MetRonnie linked a pull request Jul 8, 2024 that will close this issue
8 tasks
@MetRonnie MetRonnie modified the milestones: 8.3.2, 8.3.3 Jul 10, 2024
@wxtim wxtim modified the milestones: 8.3.3, 8.3.4 Jul 23, 2024
oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this issue Aug 7, 2024
* Do not allow users to poll for transient statuses.
* Reject invalid task states.
* Reject polling waiting and preparing tasks (not reliably pollable).
* Closes cylc#6157
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is wrong :( small
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants