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

Fix runtime environment (scheduler, validation, etc.) #5570

Merged
merged 7 commits into from
Jun 28, 2023

Conversation

hjoliver
Copy link
Member

@hjoliver hjoliver commented Jun 6, 2023

Currently in config.py we infer a run directory, and work and share sub-directories, and export associated environment variables, even when parsing a source workflow (which may not be installed) or when parsing an installed workflow but not running the scheduler (in which case work etc. may not exist).

The original motivation for this PR was:
There are use cases where src-dir needs to be distinguished from run-dir at config parsing time. Example: dependency graph constructed on the fly according to the content of some workflow sub-dir. For validation of the source workflow, the location of this content is relative to the source dir; for validation of the installed workflow (or at scheduler start-up) it must be relative to the run directory.

#5589 now provides a better solution to that problem, but in any case the exported environment should be consistent with the workflow location (as it is accessible to the template processor during parsing).

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).
  • CHANGES.md 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. (we're currently aiming all changes at 8.2.0)

@hjoliver hjoliver added the bug label Jun 6, 2023
@hjoliver hjoliver added this to the cylc-8.1.x milestone Jun 6, 2023
@hjoliver hjoliver self-assigned this Jun 6, 2023
@hjoliver hjoliver changed the base branch from master to 8.1.x June 6, 2023 22:11
@hjoliver hjoliver force-pushed the fix-scheduler-env branch 2 times, most recently from 2429a07 to 86cecb0 Compare June 6, 2023 22:45
@rosepearson
Copy link

Thanks for opening this! I have one of the examples of this use case. In my case - the Git repository containing Cylc flow also contains a folder of model parameter files. I want to run some Cylc tasks across the sites/scenarios specified in the folder without having to update the Jinga2 list each time another site/scenario is added!
Cylc's a great tool - thanks for all the ongoing maintenance and development!

@hjoliver hjoliver modified the milestones: cylc-8.1.x, cylc-8.2.0 Jun 13, 2023
@hjoliver hjoliver changed the title Fix scheduler env Fix runtime environment (scheduler, validation, etc.) Jun 20, 2023
@hjoliver hjoliver added the small label Jun 20, 2023
@hjoliver hjoliver force-pushed the fix-scheduler-env branch 2 times, most recently from d6979b7 to cb4928e Compare June 20, 2023 12:19
@hjoliver hjoliver changed the base branch from 8.1.x to master June 20, 2023 22:29
@hjoliver
Copy link
Member Author

This is ready to go. @oliver-sanders and @wxtim - plz review or reassign at your end.

@hjoliver hjoliver marked this pull request as ready for review June 21, 2023 01:38
@hjoliver hjoliver force-pushed the fix-scheduler-env branch 3 times, most recently from e24d07e to c43197c Compare June 21, 2023 09:28
@hjoliver
Copy link
Member Author

(Apologies reviewers - some stray debug code had broken the tests at the last minute ... should be fixed now!)

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.

Nothing sufficient to mark other than "Approve". Have made some suggestions.

cylc/flow/config.py Outdated Show resolved Hide resolved
cylc/flow/config.py Outdated Show resolved Hide resolved
tests/unit/test_config.py Outdated Show resolved Hide resolved
tests/unit/test_config.py Outdated Show resolved Hide resolved
tests/unit/test_config.py Outdated Show resolved Hide resolved
cylc/flow/config.py Outdated Show resolved Hide resolved
cylc/flow/config.py Outdated Show resolved Hide resolved
cylc/flow/config.py Outdated Show resolved Hide resolved
There is no run directory for a source workflow...
Add basic config doc string;
Clarify a comment;
Tweak installed check.

Co-authored-by: Tim Pillinger <[email protected]>
cylc/flow/config.py Outdated Show resolved Hide resolved
@wxtim
Copy link
Member

wxtim commented Jun 27, 2023

Changes still look good.

@oliver-sanders oliver-sanders merged commit b325d2d into cylc:master Jun 28, 2023
25 checks passed
@hjoliver hjoliver deleted the fix-scheduler-env branch June 28, 2023 21:43
wxtim added a commit to wxtim/cylc that referenced this pull request Jun 29, 2023
…c into feature.plus_s_list_of_strings

* 'feature.plus_s_list_of_strings' of github.com:wxtim/cylc:
  Update tests/unit/test_templatevars.py
  Fix runtime environment (scheduler, validation, etc.) (cylc#5570)
  Bump pypa/gh-action-pypi-publish from 1.8.6 to 1.8.7
  Fix simulation mode TypeError
  prevent tests symlinking
  subprocctx: fix xtrigger __str__
  xtriggers: improve parser
wxtim pushed a commit to wxtim/cylc that referenced this pull request Jun 29, 2023
Fix workflow environment variables when in source directory
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants