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

Fail by default if implicit/naked tasks present #4109

Merged
merged 11 commits into from
Mar 11, 2021

Conversation

MetRonnie
Copy link
Member

@MetRonnie MetRonnie commented Mar 4, 2021

These changes close #3866

Implicit tasks (task without an (empty) section in flow.cylc[runtime]) are now disallowed by default, and will raise an error on validation.

Added a setting flow.cylc[scheduler]allow implicit tasks to allow them.

Also removed the --strict option from cylc validate, as it no longer does anything.

Requirements check-list

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • Already covered by existing tests.
  • Appropriate change log entry included.
  • (master branch) I have opened a documentation PR at Update docs for implicit/naked tasks cylc-doc#207
  • No dependency changes.

@MetRonnie MetRonnie self-assigned this Mar 4, 2021
@MetRonnie MetRonnie added this to the cylc-8.0b0 milestone Mar 4, 2021
@MetRonnie MetRonnie added the config change Involves a change to global or workflow config label Mar 4, 2021
@MetRonnie MetRonnie marked this pull request as ready for review March 4, 2021 16:37
cylc/flow/config.py Outdated Show resolved Hide resolved
cylc/flow/config.py Outdated Show resolved Hide resolved
cylc/flow/scripts/validate.py Show resolved Hide resolved
tests/functional/graphing/03-filter/flow.cylc Show resolved Hide resolved
@MetRonnie MetRonnie requested a review from hjoliver March 5, 2021 12:27
@hjoliver
Copy link
Member

(All functional test batches failing).

@MetRonnie
Copy link
Member Author

😫 I think we can call this "tests pass"

image

@oliver-sanders
Copy link
Member

oliver-sanders commented Mar 11, 2021

Looking at the timestamps the last updated as 15 minutes before the timeout so this isn't just a matter of an over-zealous timeout:

Thu, 11 Mar 2021 13:27:52 GMT [18:57:52] tests/f/reload/08-cycle.t ...................................... ok    83887 ms ( 0.01 usr  0.01 sys + 97.73 cusr 29.58 csys = 127.33 CPU)
Thu, 11 Mar 2021 13:43:31 GMT Error: The action has timed out.

This is the result of cylc scan --states=all which gives us an idea of what tests were causing the issue:

cylctb-20210311T130819Z-klTO/f/jinja2/11-logging (stopped)
cylctb-20210311T130819Z-klTO/f/restart/28-execution-timeout (running)

I'll run this branch through my MacOS setup just incase...

...tests/f/jinja2/ tests/f/restart/ all pass for me

Rerunning actions to see if this is repeatable on actions...

@MetRonnie
Copy link
Member Author

It's passing 🎉 (apart from a seemingly spurious pair of codecov checks)

Copy link
Member

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

Nice, thanks @MetRonnie 👍

CHANGES.md Show resolved Hide resolved
@hjoliver hjoliver merged commit 9bc2141 into cylc:master Mar 11, 2021
wxtim added a commit to wxtim/cylc that referenced this pull request Mar 12, 2021
* 'master' of https://github.com/cylc/cylc:
  Fix a few typos (cylc#4122)
  data_store: remove prparing as a job state
  Fail by default if implicit/naked tasks present (cylc#4109)
@MetRonnie MetRonnie deleted the naked-tasks branch March 12, 2021 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
config change Involves a change to global or workflow config
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Decide on the future of naked tasks.
3 participants