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

Validate platform settings (background job runner) #4938

Merged
merged 9 commits into from
Jun 30, 2022

Conversation

hjoliver
Copy link
Member

@hjoliver hjoliver commented Jun 28, 2022

These changes close #4922

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).
  • Appropriate change log entry included.
  • (master branch) I have opened a documentation PR at Update platforms admin doc. cylc-doc#486.

@hjoliver hjoliver added the could be better Not exactly a bug, but not ideal. label Jun 28, 2022
@hjoliver hjoliver added this to the cylc-8.0rc4 milestone Jun 28, 2022
@hjoliver hjoliver requested a review from wxtim June 28, 2022 02:12
@hjoliver hjoliver self-assigned this Jun 28, 2022
@hjoliver hjoliver force-pushed the background-platform-tweak branch from 54d613c to 7cb4402 Compare June 28, 2022 03:01
@hjoliver hjoliver marked this pull request as ready for review June 28, 2022 03:02
@hjoliver hjoliver changed the title Validate platform settings. Validate platform settings (background job runner) Jun 28, 2022
@hjoliver
Copy link
Member Author

(This broke some functional tests that actually use the background job runner with multiple hosts. I have a fix in the works, will push it up later; out of time for now).

cylc/flow/platforms.py Outdated Show resolved Hide resolved
Copy link
Member

@wxtim wxtim left a comment

Choose a reason for hiding this comment

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

  • Read, seems OK.
  • Tried it out locally.

Happy that this is functionally OK, I've put a couple of thoughts about style/naming and testing in.

cylc/flow/platforms.py Outdated Show resolved Hide resolved
tests/unit/test_platforms.py Outdated Show resolved Hide resolved
@hjoliver hjoliver force-pushed the background-platform-tweak branch from d46d22e to 86f5a6b Compare June 30, 2022 00:06
@oliver-sanders oliver-sanders merged commit 1dc8bf2 into cylc:master Jun 30, 2022
@hjoliver hjoliver deleted the background-platform-tweak branch June 30, 2022 21:34
wxtim added a commit to wxtim/cylc that referenced this pull request Jul 4, 2022
* master: (30 commits)
  Log dir tidy (cylc#4836)
  Cut down on back-compat warnings, plus other tidying (cylc#4943)
  Validate platform settings (background job runner) (cylc#4938)
  clean: push error message to stderr
  Update cylc/flow/id_match.py
  Fix unintended directory stripping during cylc install (cylc#4931)
  stop cylc validate swallowing useful errors (cylc#4936)
  Improve config reference docs (cylc#4916)
  glblcfg: increase default rolling archive length
  Fix job state with pre-submitted failure
  Update tests/functional/cylc-reinstall/02-failures.t
  reinstall: require workflow ID argument
  play: upgrade --start-task's specified in legacy format (cylc#4925)
  expand schema docstring internally (cylc#4926)
  Added a new task filtering test.
  Add comment [skip ci]
  Fix optparse Options class for std options (cylc#4919)
  uid: warn if the run number is provided as a cycle
  Tweak prev.
  Fix task filtering.
  ...
@hjoliver hjoliver modified the milestones: cylc-8.0rc4, cylc-8.0.0 Jul 5, 2022
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.

background job runner and multiple hosts?
3 participants