cylc / cylc-flow

Cylc: a workflow engine for cycling systems.
https://cylc.github.io
GNU General Public License v3.0
330 stars 93 forks source link

Documentation clarification for environment variables #2962

Closed jarich closed 5 years ago

jarich commented 5 years ago

cylc_scripts

I've read https://cylc.github.io/cylc/doc/built-sphinx/appendices/suiterc-config-ref.html#suitercreference carefully, but I cannot determine which environment variables are shared with which scripts. There's a perception amongst my colleagues that the env-script only affects the environment of the pre-script, but that seems non-sensible to me... I have no idea what (if any) environment is shared with the exit-script or err-script. I've attached an image which supplies my best guess as to how the environments stack, but I have no idea if it is correct.

I will continue to attempt to poke around to see what is available, but perhaps something like (a correct version) of my image linked above would help illustrate this in your documentation.

hjoliver commented 5 years ago

@jarich - sorry I missed this before emailing you. The thing to note (and I guess we need to document this better!) is that the task job script is "just a shell script" that is essentially constructed by bunging all the xxx-script items and env var definitions together in the right order (albeit not so obvious these days by looking at a job script because the bits of it are now defined as shell functions that are called from the boilerplate lib/cylc/job.sh). So there is no deliberate passing of environment variables to different scripts as such, it's just order that matters. The order is:

So init-script can't use any suite or task vars. env-script can use suite vars, and can effect task vars all other script segments can use all vars

hjoliver commented 5 years ago

So in your diagram, the env-script box should be to the left of the big box, and the exit and err scripts should be inside the big box.

jarich commented 5 years ago

Cheers, I'll update the diagram and send you a copy reflecting this advice.

J

On Thu, 21 Feb. 2019, 18:52 Hilary James Oliver, notifications@github.com wrote:

So in your diagram, the env-script box should be to the left of the big box, and the exit and err scripts should be inside the big box.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/cylc/cylc/issues/2962#issuecomment-465897634, or mute the thread https://github.com/notifications/unsubscribe-auth/AAA0eWDK4yM8vMLYN4FpioGeKXUKQ5g1ks5vPlBXgaJpZM4bGgub .

hjoliver commented 5 years ago

You can test this sort of thing pretty quickly:

[scheduling]
  [[dependencies]]
      graph = "foo"
[runtime]
  [[foo]]
      init-script = "echo [INIT-SCRIPT] FOO is ${FOO:-undefined}"
      env-script = """
        echo [ENV-SCRIPT-1] FOO is ${FOO:-undefined}
        FOO=bar
        echo [ENV-SCRIPT-2] FOO is ${FOO:-undefined}"""
      pre-script = "echo [PRE-SCRIPT] FOO is ${FOO:-undefined}"
      script = "echo [SCRIPT] FOO is ${FOO:-undefined}"
      post-script = "echo [POST-SCRIPT] FOO is ${FOO:-undefined}"
      exit-script = "echo [EXIT-SCRIPT] FOO is ${FOO:-undefined}"
      err-script = "echo [ERR-SCRIPT] FOO is ${FOO:-undefined}"
      [[[environment]]]
         FOO = ${FOO:-foo}

Run suite above as "test", then:

$ cylc cat-log -f o test foo.1   

[INIT-SCRIPT] FOO is undefined

Suite    : test
Task Job : 1/foo/01 (try 1)
User@Host: oliverh@osboxes

[ENV-SCRIPT-1] FOO is undefined
[ENV-SCRIPT-2] FOO is bar
[PRE-SCRIPT] FOO is bar
[SCRIPT] FOO is bar
[POST-SCRIPT] FOO is bar

2019-02-21T21:02:48+13:00 INFO - started
2019-02-21T21:02:48+13:00 INFO - succeeded

[EXIT-SCRIPT] FOO is bar

Then again with FOO=bar commented out of env-script:

$ cylc cat-log -f o test foo.1  

[INIT-SCRIPT] FOO is undefined

Suite    : test
Task Job : 1/foo/01 (try 1)
User@Host: oliverh@osboxes

[ENV-SCRIPT-1] FOO is undefined
[ENV-SCRIPT-2] FOO is undefined
[PRE-SCRIPT] FOO is foo
[SCRIPT] FOO is foo
[POST-SCRIPT] FOO is foo

2019-02-21T21:07:11+13:00 INFO - started
2019-02-21T21:07:12+13:00 INFO - succeeded

[EXIT-SCRIPT] FOO is foo
hjoliver commented 5 years ago

I'll leave this issue open as a reminder to amend the documentation a bit.

hjoliver commented 5 years ago

Oh, also in your diagram, "task env vars" is the same thing as "environment inherited from families/root". (unless you're referring to variables defined inside your own script segment, which is nothing to do with Cylc).

hjoliver commented 5 years ago

Should probably be documented under the "Task Implementation -> Task Job Scripts" section.

hjoliver commented 5 years ago

(Note there are moves afoot to execute user-defined scripting in subshells - #2885 - depending on how we do that it could stop one script segnment affecting another through the environment).

jarich commented 5 years ago

cylc_scripts

I think this matches your description, so I'm going to hope it's more correct.

I am also happy to share the http://draw.io XML if that would be handy (for example if you want to make edits before using this diagram yourself.)

jarich commented 5 years ago

I guess that technically, what I've said is the task environment should be a box inside the familes/root box, given that individual task environments may override the values they've inherited. But I think we can hope that our users will understand that.

hjoliver commented 5 years ago

@jarich - that looks right. Yes, please share the source, I'll use it in the User Guide (with your permission).

jarich commented 5 years ago

cylc_environments.zip

Here you go. Just unzip it and open it as an existing diagram in http://www.draw.io

All rights surrendered, and I promise not to be upset if you decide to completely overhaul it or replace it with something created in some other format.

sadielbartholomew commented 5 years ago

Just jumping in here (briefly) to say that I think the documentation will really benefit from diagrams such as this one, even in the case where the original written content includes all the equivalent information somewhere, since visual information can be much easier & quicker to comprehend & would reinforce & complement the text, so thank you @jarich! This (or something similar if we don't go for this exact version) will be a really good addition I think.

kinow commented 5 years ago

Maybe we need to include this one in the milestone next-release @hjoliver ?

matthewrmshin commented 5 years ago

(No pressure for @hjoliver!)

hjoliver commented 5 years ago

I'll do it.

hjoliver commented 5 years ago

@jarich - on reflection I decided to redo your diagram to emphasize that the xxx-script items and environment section are just concatenated to create a single unified script (no sub-shells or distinct processes involved):

image

So you can mess with the environment at any point in the chain, and if you do that it can effect everything that comes after.

I don't think your otherwise-excellent diagram quite conveys this, because it omits the full picture (e.g. you could also set new environment variables in script and reference them in post-script...)

Hope that's OK with you? (If so, I'll put up a doc change pull request shortly).