-
Notifications
You must be signed in to change notification settings - Fork 93
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
clear cylc options at reload #6065
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1262,11 +1262,17 @@ def _configure_contact(self) -> None: | |
|
||
def load_flow_file(self, is_reload=False): | ||
"""Load, and log the workflow definition.""" | ||
# Local workflow environment set therein. | ||
# Allow -S and -D to take effect in Cylc VR. | ||
# https://github.com/cylc/cylc-flow/issues/5968 | ||
self.options.rose_template_vars = [] | ||
self.options.defines = [] | ||
if is_reload: | ||
# If the workflow is reloaded clear any existing rose options | ||
# The reload command doesn't have the ability to set these but | ||
# if the user has used VR to re-install before reload they will | ||
# expect the changed values not the ones stored on the scheduler. | ||
# Note: Does NOT apply to reload on restart, because the play | ||
# command used to restart can have template variables attached. | ||
# See https://github.com/cylc/cylc-flow/pull/5996 | ||
Comment on lines
+1270
to
+1272
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This explanation doesn't make much sense to me as the play command used to start the workflow in the first place can also have template variables attached? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you do
Do you want foo=1 or 2 or 3? This is also safe against
because the restart involves re-reading the rose config files too. |
||
self.options.rose_template_vars = [] | ||
self.options.defines = [] | ||
self.options.opt_conf_keys = [] | ||
return WorkflowConfig( | ||
self.workflow, | ||
self.flow_file, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit confused
cylc vr running-workflow -S
(reinstall + reload), how are the changed values getting through to the scheduler then?cylc vr stopped-workflow -S
(reinstall + restart), then there is no reload, right?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reinstall updates the
rose-suite-cylc-install.conf
which the reload reads. The reload doesn't have the-S
option so clearing that here only affects options set on the scheduler by an earlier Cylc play.Your question has however revealed a horrid case where this doesn't work:
Do you think that the correct options might be to allow Cylc Reload to set
-S
and it's friends, becuase to me that looks like the simplest way of doing things?Oliver described it as an implicit reload.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does happen with this example, and what is meant to happen?