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

Further cylc wrapper improvements? #4555

Open
hjoliver opened this issue Dec 15, 2021 · 10 comments
Open

Further cylc wrapper improvements? #4555

hjoliver opened this issue Dec 15, 2021 · 10 comments
Labels
question Flag this as a question for the next Cylc project meeting.
Milestone

Comments

@hjoliver
Copy link
Member

hjoliver commented Dec 15, 2021

Should we get the wrapper to:

  • automatically invoke cylc-7.9.5 if cylc review is called in a Cylc 8 environment? DONE
  • list available Cylc versions?
  • handle site config locations?
  • make the prefix and the name of the default version configurable.
@hjoliver hjoliver added the question Flag this as a question for the next Cylc project meeting. label Dec 15, 2021
@hjoliver hjoliver added this to the cylc-8.0.0 milestone Dec 15, 2021
@hjoliver
Copy link
Member Author

hjoliver commented Dec 15, 2021

list available versions

Quick hack:

if [[ $1 == "list-versions" ]]; then
   ls -l "${CYLC_HOME_ROOT}" | awk '{print $9,$10,$11}'
   exit 0
fi

result:

$ cylc list-versions
                                                        
7.9.1 
7.9.2 
7.9.5 
8.0b3 
cylc -> 7.9.1  

@hjoliver
Copy link
Member Author

hjoliver commented Dec 15, 2021

cylc review invocation

Quick hack:

if [[ "${CYLC_VERSION:-x}" =~ ^8 ]]; then
   if [[ "${1:-}" == "review" ]]; then
      echo "Starting Cylc 7.9.5 'cylc review' for Cylc 8 logs."
      CYLC_VERSION=7.9.5  # latest Cylc 7 "cylc review" required for Cylc 8.
   fi
fi

@hjoliver
Copy link
Member Author

hjoliver commented Dec 15, 2021

[UPDATE:] this is a niche problem for me due to the fact that one cylc 7 installation that I do not have write access to is missing its global config, so I wanted to try to put cylc 7 configs under the same hierarchy as cylc 8 (where I do have write access) for use via the new wrapper ... but we're stuck with the CONF variables as-is, so I can either stick with the workaround below, or get hpc admins to fix the broken Cylc 7 version.

site config locations

The wrapper could be a convenient place to set CYLC_SITE_CONF_PATH to a non-standard location (for instance, /etc is not normally (ever?) on the shared filesystem?). Easier set up for the cylc admin, because users only need the wrapper in their executable search path, no other environment settings required.

However, I came across a problem with this at NIWA, getting the wrapper to work with both Cylc 8 and Cylc 7 site configs in the same directory hierarchy, because Cylc 7's CYLC_CONF_PATH is more explicit than Cylc 8's CYLC_SITE_CONF_PATH. Had to do this in the wrapper:

if [[ "${CYLC_ENV_NAME:-x}" =~ ^8 || "${CYLC_VERSION:-x}" =~ ^8 ]]; then  # NIWA
   # Cylc 8 site config
   export CYLC_SITE_CONF_PATH="${CYLC_SITE_CONF_PATH:-/opt/nesi/share/cylc/etc}"  # /flow/VERSION assumed
   unset CYLC_CONF_PATH  # and this interferes with Cylc 8 too
else
   # Cylc 7 site config
   export CYLC_CONF_PATH="${CYLC_CONF_PATH:-/opt/nesi/share/cylc/etc/flow/7}"  # /flow/VERSION required
fi

@dpmatthews
Copy link
Contributor

List available versions:

  • This wouldn't work for us because we install the Cylc environments into a directory containing lots of other things.
  • For Cylc 8 it's environments rather than versions.
  • There's also CYLC_HOME_ROOT_ALT to consider.
  • This would add a command not listed by cylc help.

cylc review invocation:

  • I was planning on adding this kind of support for rose edit.
  • I hadn't really considered cylc review because I didn't think it is widely used on the command line but I've no objection if you think it's worth it.

site config locations:

  • We just set CYLC_SITE_CONF_PATH and ROSE_SITE_CONF_PATH (as indicated in the current wrapper).
  • I don't think we need to consider CYLC_CONF_PATH in general. The Cylc 7 installation instructions never suggested using it (it wasn't really an option when supporting multiple versions).

@wxtim
Copy link
Member

wxtim commented Dec 15, 2021

Generally in agreement with @dpmatthews , but I question

This wouldn't work for us because we install the Cylc environments into a directory containing lots of other things.

Wouldn't this fix the issue for us?

-    ls -l "${CYLC_HOME_ROOT}" | awk '{print $9,$10,$11}'
+    find "${CYLC_HOME_ROOT}/cylc*" -maxdepth 0 | awk -F '/' '{print $NF}'

@hjoliver
Copy link
Member Author

hjoliver commented Dec 15, 2021

List available versions:

This wouldn't work for us because we install the Cylc environments into a directory containing lots of other things.

So do we (well, some other things) - I simplified my example above, but it's easy to exclude other things.

For Cylc 8 it's environments rather than versions.

Yes, but for centrally installed environments named by the release version, the wrapper works for Cylc 8 with CYLC_VERSION, which is easier for normal users. And we could even make it list environments that don't conform to that naming convention, of course.

There's also CYLC_HOME_ROOT_ALT to consider.

Easy to add that in.

This would add a command not listed by cylc help.

Yes but the wrapper could intercept cylc help commands to deal with that.

@hjoliver
Copy link
Member Author

IMO listing available versions easily would be valuable. A lot of users at my site stick with old versions for ever because they don't know (or forgot) which newer versions are available.

@hjoliver
Copy link
Member Author

hjoliver commented Dec 15, 2021

cylc review invocation

At NIWA we can't have a central cylc review server for everyone because of the private "project directory" problem. This will ultimately be solved properly by the Cylc 8 UI Server serving workflow logs.

@hjoliver
Copy link
Member Author

site config locations

Yeah the problem is CYLC_CONF_PATH is version specific, rather than the top of a multi-version hierarchy. And we still need that occasionally in Cylc 8 (e.g. for functional tests). So I guess I just have to work around that as above, or get hpc admin to fix the missing global.rc outside of my config directory.

I'll cross out this one from the list above.

dpmatthews added a commit to dpmatthews/cylc-flow that referenced this issue Feb 3, 2022
… flow version (e.g. cylc-8.0.0-1)

Ensure workflows always run under fixed environments rather than symlinked versions which could change
Provide legacy support for rose edit & cylc review (partially address cylc#4555)
dpmatthews added a commit to dpmatthews/cylc-flow that referenced this issue Feb 4, 2022
… flow version (e.g. cylc-8.0.0-1)

Ensure workflows always run under fixed environments rather than symlinked versions which could change
Provide legacy support for rose edit & cylc review (partially address cylc#4555)
dpmatthews added a commit to dpmatthews/cylc-flow that referenced this issue Feb 4, 2022
… flow version (e.g. cylc-8.0.0-1)

Ensure workflows always run under fixed environments rather than symlinked versions which could change
Provide legacy support for rose edit & cylc review (partially address cylc#4555)
dpmatthews added a commit to dpmatthews/cylc-flow that referenced this issue Feb 7, 2022
… flow version (e.g. cylc-8.0.0-1)

Ensure workflows always run under fixed environments rather than symlinked versions which could change
Provide legacy support for rose edit & cylc review (partially address cylc#4555)
@dpmatthews
Copy link
Contributor

automatically invoke cylc-7.9.5 if cylc review is called in a Cylc 8 environment?

Fixed in #4655. However a new enhancement was raised: make the prefix and the name of the default version configurable.
#4655 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Flag this as a question for the next Cylc project meeting.
Projects
None yet
Development

No branches or pull requests

4 participants