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

Changed behaviour in handling template variables passed via cli #6058

Closed
jamesgrimmett opened this issue Apr 8, 2024 · 7 comments · Fixed by cylc/cylc-rose#312
Closed

Changed behaviour in handling template variables passed via cli #6058

jamesgrimmett opened this issue Apr 8, 2024 · 7 comments · Fixed by cylc/cylc-rose#312
Assignees
Labels
bug Something is wrong :(
Milestone

Comments

@jamesgrimmett
Copy link

Description

I think that there has been a change in behaviour in handling template variables passed to workflows via the -S cli option in v8.2.5.
In v8.2.3, a template variable passed via -S would override that variable set in optional config files. In v8.2.5, it looks like this behaviour is reversed, and the setting in optional config will override the setting passed to -S.

Reproducible Example

Example test suite containing;

test_workflow
├── flow.cylc
├── rose-suite.conf
├── opt
    └── rose-suite-common.conf

[flow.cylc]

#!jinja2
[scheduler]
   allow implicit tasks=True

[scheduling]
  initial cycle point = {{ START_CYCLE }}
  final cycle point = {{ END_CYCLE }}
  [[ graph]]
    R1 = cold_start
    T00 = """
      cold_start => a
    a => b
    """

[opt/rose-suite-common.conf]

[template variables]
START_CYCLE="20240101T00"
END_CYCLE="20240102T00"

Variables passed using -S are ignored, and the workflow runs with the settings from opt/rose-suite-common.conf;

$ cylc --version
8.2.5

$ cylc vip -O common -S 'START_CYCLE="20230102"' -S 'END_CYCLE="20230102"' .

$ ls ~/cylc-run/test_workflow/run1/log/job/
20240101T0000Z  20240102T0000Z

Expected Behaviour

The setting passed via -S overrides other config

$ cylc --version
8.2.3

$ cylc vip -O common -S 'START_CYCLE="20230102"' -S 'END_CYCLE="20230102"' .

$ ls ~/cylc-run/test_workflow/run1/log/job/
20230102T0000Z

I seems that I get the behaviour that I'm looking for if I use -s instead of -S, so I could potentially use that as a workaround, but I don't have a good understanding of the difference between these two options. I note that -s will currently not allow variables to be updated on reload (#5965 / #6045), are there any other significant differences between these two options (i.e., -s vs -S)?

$ cylc --version
8.2.5

$ cylc vip -O common -s 'START_CYCLE="20230102"' -s 'END_CYCLE="20230102"' .

$ ls ~/cylc-run/test_workflow/run1/log/job/
20230102T0000Z
@jamesgrimmett jamesgrimmett added the bug Something is wrong :( label Apr 8, 2024
@jamesgrimmett jamesgrimmett changed the title Changed behaviour in template variables passed via cli Changed behaviour in handling template variables passed via cli Apr 8, 2024
@hjoliver
Copy link
Member

hjoliver commented Apr 8, 2024

.. I get the behaviour that I'm looking for if I use -s instead of -S, ... but I don't have a good understanding of the difference between these two options.

The -S option is injected into Cylc by the cylc-rose plugin, so I'll leave this one to the UK Rose team overnight...
They'll probably want to move this issue to the cylc-rose repository.

@wxtim
Copy link
Member

wxtim commented Apr 8, 2024

-s is a native Cylc feature: As such it ought to be safe as a work-around.

I will examine this when I get into the office.

@oliver-sanders
Copy link
Member

We recently changed this: cylc/cylc-rose#302

Shouldn't be related?

@wxtim
Copy link
Member

wxtim commented Apr 8, 2024

We recently changed this: cylc/cylc-rose#302

Shouldn't be related?

I don't think* that should be causing this change.

* To a very high level of certainty.

@wxtim wxtim transferred this issue from cylc/cylc-flow Apr 8, 2024
@wxtim wxtim self-assigned this Apr 8, 2024
@wxtim
Copy link
Member

wxtim commented Apr 8, 2024

I can confirm that this

  • This looks like a bug.
  • It applies to both cylc install && cylc play and to cylc vip (i.e. it's not a side effect of something VIP is doing).
  • It is happening with the most recent versions of Cylc and Cylc-Rose.

Results from multiple runs suggest that this may be a change to Cylc.

===============
8.2.4+1.3.x
...............
CLI
===============
8.2.5+1.3.0
...............
opt_file
===============
8.2.5+1.3.2
...............
opt_file
===============
8.2.5+1.3.3
...............
opt_file
===============
8.2.5+1.3.x
...............
opt_file
===============
8.2.x+1.3.x
...............
opt_file
===============
master+1.3.x
...............
opt_file

Cause of the bug!

#5996

@wxtim
Copy link
Member

wxtim commented Apr 8, 2024

Overview of solution

For Cylc 8.2.x:

#6059 - Cleaned all 3 config items on this branch.

For Cylc 8.3.0 and Cylc Rose 1.4.0 (master branches in both repos)

#6060 - remove all the cleaning code from Cylc-flow
cylc/cylc-rose#308 - added the option removal to Cylc Rose.

@MetRonnie MetRonnie added this to the 8.2.6 milestone Apr 9, 2024
@MetRonnie MetRonnie linked a pull request Apr 23, 2024 that will close this issue
8 tasks
@oliver-sanders
Copy link
Member

oliver-sanders commented May 1, 2024

Closed by cylc/cylc-rose#312

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is wrong :(
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants