-
Notifications
You must be signed in to change notification settings - Fork 94
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
Allow 'cylc reload' to reload global configuration #6509
base: master
Are you sure you want to change the base?
Changes from 5 commits
3e9e9a6
656c275
2ae8bb3
9b7c1e8
7f3bd41
7be6be5
d6fd822
172537d
2236a70
49bf3de
257e817
557f74d
b4542e1
7f5bfb1
8c93385
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 |
---|---|---|
|
@@ -88,6 +88,7 @@ | |
RunMode, | ||
StopMode, | ||
) | ||
from cylc.flow.cfgspec.glbl_cfg import glbl_cfg | ||
|
||
|
||
if TYPE_CHECKING: | ||
|
@@ -329,7 +330,7 @@ | |
|
||
|
||
@_command('reload_workflow') | ||
async def reload_workflow(schd: 'Scheduler'): | ||
async def reload_workflow(schd: 'Scheduler', reload_global: bool = False): | ||
"""Reload workflow configuration.""" | ||
yield | ||
# pause the workflow if not already | ||
|
@@ -363,6 +364,25 @@ | |
# give commands time to complete | ||
sleep(1) # give any remove-init's time to complete | ||
|
||
if reload_global: | ||
# Reload global config if requested | ||
LOG.info("Reloading the global configuration.") | ||
try: | ||
glbl_cfg(reload=True) | ||
except (ParsecError, CylcConfigError) as exc: | ||
if cylc.flow.flags.verbosity > 1: | ||
# log full traceback in debug mode | ||
LOG.exception(exc) | ||
LOG.critical( | ||
f'Reload failed - {exc.__class__.__name__}: {exc}' | ||
'\nThis is probably due to an issue with the new' | ||
' configuration.' | ||
'\nTo continue with the pre-reload config, un-pause the' | ||
' workflow.' | ||
'\nOtherwise, fix the configuration and attempt to reload' | ||
' again.' | ||
) | ||
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. The way this is implemented, if the global config fails to reload, we log the error, but continue and reload the workflow config. I see So I would expect it to bail at this point. I.E, If the global reload fails, I would expect the logic to skip straight to this block of logic at the end of the routine: # resume the workflow if previously paused
schd.reload_pending = False
schd.update_data_store() # update workflow status msg
schd._update_workflow_state()
if not was_paused_before_reload:
schd.resume_workflow()
schd.process_workflow_db_queue() # see #5593 That does leave us with the situation where the global config does reload, but the workflow config doesn't (e.g. a validation error). Currently, the global config change will not roll back in this situation, but we could get it to do that something along the lines of: if reload_global:
...
try:
cfg = glbl_cfg(cached=False)
except ...:
...
...
# reload the workflow definition
...
glbl_cfg().reload(cfg) (would require a reload method on the |
||
|
||
# reload the workflow definition | ||
schd.reload_pending = 'loading the workflow definition' | ||
schd.update_data_store() # update workflow status msg | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1886,6 +1886,10 @@ class Meta: | |
class Arguments: | ||
workflows = graphene.List(WorkflowID, required=True) | ||
|
||
reload_global = Boolean( | ||
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. Unfortunately, this is a breaking change that will cause errors when Somewhat surprisingly, we don't yet have a working pattern for adding new fields to mutations. It's one of the things on the TODO list that we haven't found time for yet!
I think we can get around both issues by marking this field as optional ( This way the mutation should remain forward and backward compatible so long as the Sadly, we don't have any automated inter-version testing as yet (another one for the TODO list) so this will need to be tested manually for now. |
||
default_value=False, | ||
description="Also reload global config") | ||
|
||
Comment on lines
+1922
to
+1925
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. |
||
result = GenericScalar() | ||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -69,10 +69,12 @@ | |
|
||
MUTATION = ''' | ||
mutation ( | ||
$wFlows: [WorkflowID]! | ||
$wFlows: [WorkflowID]!, | ||
$reloadGlobal: Boolean, | ||
) { | ||
reload ( | ||
workflows: $wFlows | ||
reloadGlobal: $reloadGlobal | ||
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. [In relation to above comment] I think we can avoid inter-version interoperability issues providing we omit the new |
||
) { | ||
result | ||
} | ||
|
@@ -87,6 +89,12 @@ | |
multiworkflow=True, | ||
argdoc=[WORKFLOW_ID_MULTI_ARG_DOC], | ||
) | ||
|
||
parser.add_option( | ||
"-g", "--global", | ||
help="also reload global configuration.", | ||
action="store_true", default=False, dest="reload_global") | ||
|
||
return parser | ||
|
||
|
||
|
@@ -97,6 +105,7 @@ | |
'request_string': MUTATION, | ||
'variables': { | ||
'wFlows': [workflow_id], | ||
'reloadGlobal': options.reload_global, | ||
} | ||
} | ||
|
||
|
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.
This will update the workflow status message (shown in the GUI toolbar) to reflect what Cylc is doing:
It's useful info incase the operation gets stuck with a filesystem issue, or incase loading the config takes a long time for whatever reason.