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

cylc set stuffs #44

Merged
merged 2 commits into from
Feb 28, 2024
Merged

Conversation

oliver-sanders
Copy link

This fixes a test which was broken by log format changes but isn't run in CI.

It also adds some extra validation to the cylc set command to catch some likely user errors.

* Check that task/cycle selectors are not provided.
  * It might feel natural to users to specify the task output as part of
  * the task ID, but this won't work, so catch it early.
* Prohibit setting the (non-existent) "waiting" output
  * Provide a more helpful error for Cylc 7 users who are likely to try
    this.
  * Also blacklist the "waiting" task state name from use in custom
    outputs.
Comment on lines +328 to +331
if "waiting" in outputs:
raise InputError(
"Tasks can not be set to waiting, use a new flow to re-run"
)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pre-empting people trying to cylc set --out=waiting.

Comment on lines +379 to +383
if tokens['task_sel']:
raise InputError(SELECTOR_ERROR.format(
tokens['task'],
tokens['task_sel'],
))
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I absent-mindedly tried running a command like this cylc set workflow//cycle/task:output rather than cylc set workflow//cycle/task --out=output.

The CLI accepted this, but it didn't match any tasks because it merged the output in with the task name.

Comment on lines +44 to +45
poll_grep_workflow_log -E '19700101T0000Z/t1/01:submitted.* => running'
poll_grep_workflow_log -E '19700101T0000Z/t1/01:running.* => failed'
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test got missed as it isn't run in CI

Copy link
Owner

@hjoliver hjoliver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hjoliver hjoliver merged commit b75875e into hjoliver:cylc-set-task Feb 28, 2024
24 of 27 checks passed
@oliver-sanders oliver-sanders deleted the cylc-set-task-2 branch February 28, 2024 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants