Skip to content

Conversation

@oliver-sanders
Copy link
Member

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 if present).
  • Tests are included (or explain why tests are not needed).
  • Changelog entry included if this is a change that can affect users
  • Cylc-Doc pull request opened if required at cylc/cylc-doc/pull/XXXX.
  • If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

* Closes cylc#7029
* This reduces the pressure on cylc#7001 (polling is not attempted if a
  platform runs out of hosts until the next run of reset-bad hosts) by
  making it easier for operators to reset bad hosts and recover their
  workflow in the event of platform outages.
Comment on lines +997 to +1008
if continue_if_no_good_hosts:
# no hosts available for this platform
# -> reset the bad hosts and try again
self.task_events_mgr.reset_bad_hosts()
host = get_host_from_platform(
platform, bad_hosts=self.bad_hosts
)
else:
ctx.err = f'No available hosts for {platform["name"]}'
LOG.debug(ctx)
callback_255(ctx, itasks)
continue
Copy link
Member

Choose a reason for hiding this comment

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

Both branches here continue to the next iteration of the for loop. host is not used in the first branch. Did you mean to remove the else condition on the line below?

Copy link
Member

Choose a reason for hiding this comment

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

Mildly concerned that if Ronnie is right (I think he is) that the test isn't failing....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

could be better Not exactly a bug, but not ideal.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants