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

Clean other remotes after error. #4950

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hjoliver
Copy link
Member

@hjoliver hjoliver commented Jun 30, 2022

These changes close #4935

If cylc clean fails to clean a remote platform, continue on to other remote platforms but do not clean locally.

@oliver-sanders - you've suggested going further than this #4935 (comment) - but I think that's a potential follow-up. This quick-fix will be a significant improvement as-is - OK?

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).
  • Already covered by existing tests.
  • Does not need tests (why?).
  • Appropriate change log entry included.
  • No change log entry required (why? e.g. invisible to users).
  • (master branch) I have opened a documentation PR at cylc/cylc-doc/pull/XXXX.
  • (7.8.x branch) I have updated the documentation in this PR branch.
  • No documentation update required.

@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 commented Jun 30, 2022

you've suggested going further than this #4935 (comment) - but I think that's a potential follow-up. This quick-fix will be a significant improvement as-is - OK?

I don't think #4935 is much of an issue (at least not much of an 8.0.0 issue) so happy with whatever.

(the "broken platform" issue mostly impacts developers and if clean fails it makes little difference to uses how many platforms it cleaned before bailing, either way they are going to have to re-run the command later).

@oliver-sanders
Copy link
Member

LGTM; just needs a test.

@hjoliver
Copy link
Member Author

I don't think #4935 is much of an issue (at least not much of an 8.0.0 issue) so happy with whatever.

I agree it's a small issue. #4949 is more important, and this sort of caused that (i.e., a remote clean failure in one workflow prevented cleaning of a bunch of others at the same time). However, it still a bug (cleaning a valid remote should not depend on whether or not we try to clean an invalid one first) and easy to fix.

@hjoliver
Copy link
Member Author

hjoliver commented Jul 1, 2022

However, as this one is not very important and it's not instantly clear to me how to test it, I'll bump it back to 8.x

@hjoliver hjoliver modified the milestones: cylc-8.0rc4, cylc-8.x Jul 1, 2022
@oliver-sanders oliver-sanders modified the milestones: cylc-8.x, cylc-8.1.0 Aug 15, 2022
@oliver-sanders oliver-sanders modified the milestones: cylc-8.1.0, cylc-8.2.0 Oct 18, 2022
@oliver-sanders oliver-sanders modified the milestone: cylc-8.2.0 May 2, 2023
@oliver-sanders oliver-sanders modified the milestones: cylc-8.2.0, cylc-8.3.0 Jul 11, 2023
@oliver-sanders oliver-sanders modified the milestones: cylc-8.3.0, cylc-8.x Oct 12, 2023
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.

cylc clean: continue if remote clean fails?
2 participants