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

Don't abort if cleaning multiple workflows. #4949

Merged
merged 4 commits into from
Jul 8, 2022

Conversation

hjoliver
Copy link
Member

@hjoliver hjoliver commented Jun 30, 2022

These changes close #4934

@oliver-sanders - any problem with nailing this now?

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).
  • Applied any dependency changes to both setup.cfg and conda-environment.yml.
  • Appropriate tests are included (unit and/or functional).
  • No change log entry required (why? e.g. invisible to users).
  • No documentation update required.

@hjoliver hjoliver added the bug Something is wrong :( label Jun 30, 2022
@hjoliver hjoliver added this to the cylc-8.0rc4 milestone Jun 30, 2022
@hjoliver hjoliver self-assigned this Jun 30, 2022
@oliver-sanders
Copy link
Member

@oliver-sanders - any problem with nailing this now?

Not at all, but happy to wait for 8.x for this.

@hjoliver
Copy link
Member Author

hjoliver commented Jun 30, 2022

@oliver-sanders - any problem with nailing this now?

Not at all, but happy to wait for 8.x for this.

But it's a trivial change, done already, and fixes a bug - so let's get it in! [I want to nail as many easy-to-fix user-facing bugs as possible before 8.0]

@hjoliver hjoliver mentioned this pull request Jun 30, 2022
12 tasks
init_clean(workflow, opts)
try:
init_clean(workflow, opts)
except Exception as exc:
Copy link
Member Author

Choose a reason for hiding this comment

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

(Probably one of the few situations where it's fine to catch all exceptions IMO - but can try to be specific if needed.)

Copy link
Member

Choose a reason for hiding this comment

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

I think this is fine here, sometimes you need to catch anything.

@hjoliver hjoliver marked this pull request as ready for review July 1, 2022 02:29
@oliver-sanders
Copy link
Member

Looks fine, tried out manually, test failed CI and needs some bodging.

@hjoliver
Copy link
Member Author

hjoliver commented Jul 4, 2022

CI fixed - the new test should have had a run_fail on the clean command.

@hjoliver hjoliver modified the milestones: cylc-8.0rc4, cylc-8.0.0 Jul 5, 2022
@datamel datamel self-requested a review July 8, 2022 12:48
Copy link
Contributor

@datamel datamel left a comment

Choose a reason for hiding this comment

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

Checked out, tests run. No problems spotted. Thanks @hjoliver.

@datamel datamel merged commit 0113de8 into cylc:master Jul 8, 2022
@hjoliver hjoliver deleted the clean-tweak branch July 20, 2022 22:16
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 this pull request may close these issues.

cylc clean: don't bail early on a list of workflows
3 participants