-
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 all 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 |
---|---|---|
@@ -0,0 +1 @@ | ||
Added --global flag to 'cylc reload' | ||
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -108,6 +108,13 @@ | |||||
|
||||||
Not to be confused with :cylc:conf:`flow.cylc[scheduling]`. | ||||||
|
||||||
.. note:: | ||||||
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. FYI: This note will only appear in the workflow config reference, not the global config reference due to the default_for function which only includes the first paragraph. |
||||||
|
||||||
The majority of scheduler settings affect the server and cannot be reloaded | ||||||
with ``cylc reload --global``, the server must be stopped and restarted for | ||||||
changes to take effect except for the sections [mail] and [events] which | ||||||
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. Little bit of Cylc Sphinx magic, use single backquotes (or the log role
Suggested change
This generates links, but also runs the references through the internal consistency checker to make sure they stay valid. |
||||||
provide workflow defaults. | ||||||
|
||||||
.. versionchanged:: 8.0.0 | ||||||
|
||||||
{REPLACES}``[cylc]`` | ||||||
|
@@ -1171,6 +1178,11 @@ def default_for( | |||||
Symlinks from the the standard ``$HOME/cylc-run`` locations will be | ||||||
created. | ||||||
|
||||||
.. note:: | ||||||
|
||||||
Once a platform has been installed and symlinks created they | ||||||
cannot be modified for that run. | ||||||
|
||||||
.. versionadded:: 8.0.0 | ||||||
"""): | ||||||
with Conf('<install target>', desc=dedent(""" | ||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -85,6 +85,7 @@ | |||||||||||
from cylc.flow.parsec.exceptions import ParsecError | ||||||||||||
from cylc.flow.run_modes import RunMode | ||||||||||||
from cylc.flow.task_id import TaskID | ||||||||||||
from cylc.flow.cfgspec.glbl_cfg import glbl_cfg | ||||||||||||
from cylc.flow.workflow_status import StopMode | ||||||||||||
|
||||||||||||
|
||||||||||||
|
@@ -327,7 +328,7 @@ async def remove_tasks( | |||||||||||
|
||||||||||||
|
||||||||||||
@_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 | ||||||||||||
|
@@ -361,6 +362,25 @@ async def reload_workflow(schd: 'Scheduler'): | |||||||||||
# 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.") | ||||||||||||
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 will update the workflow status message (shown in the GUI toolbar) to reflect what Cylc is doing:
Suggested change
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. |
||||||||||||
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 |
---|---|---|
|
@@ -1919,6 +1919,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 |
---|---|---|
|
@@ -60,19 +60,34 @@ | |
from cylc.flow.option_parsers import ( | ||
WORKFLOW_ID_MULTI_ARG_DOC, | ||
CylcOptionParser as COP, | ||
OptionSettings, | ||
) | ||
from cylc.flow.terminal import cli_function | ||
|
||
if TYPE_CHECKING: | ||
from optparse import Values | ||
|
||
|
||
RELOAD_OPTIONS = [ | ||
OptionSettings( | ||
['-g', '--global'], | ||
help='also reload global configuration.', | ||
action="store_true", | ||
default=False, | ||
dest="reload_global", | ||
sources={'reload'} | ||
), | ||
] | ||
|
||
|
||
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 +102,9 @@ def get_option_parser(): | |
multiworkflow=True, | ||
argdoc=[WORKFLOW_ID_MULTI_ARG_DOC], | ||
) | ||
for option in RELOAD_OPTIONS: | ||
parser.add_option(*option.args, **option.kwargs) | ||
|
||
return parser | ||
|
||
|
||
|
@@ -97,6 +115,7 @@ async def run(options: 'Values', workflow_id: str): | |
'request_string': MUTATION, | ||
'variables': { | ||
'wFlows': [workflow_id], | ||
'reloadGlobal': options.reload_global, | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
#!/usr/bin/env bash | ||
# THIS FILE IS PART OF THE CYLC WORKFLOW ENGINE. | ||
# Copyright (C) NIWA & British Crown (Met Office) & Contributors. | ||
# | ||
# This program is free software: you can redistribute it and/or modify | ||
# it under the terms of the GNU General Public License as published by | ||
# the Free Software Foundation, either version 3 of the License, or | ||
# (at your option) any later version. | ||
# | ||
# This program is distributed in the hope that it will be useful, | ||
# but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
# GNU General Public License for more details. | ||
# | ||
# You should have received a copy of the GNU General Public License | ||
# along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
#------------------------------------------------------------------------------- | ||
# Test reloading global configuration | ||
. "$(dirname "$0")/test_header" | ||
set_test_number 9 | ||
|
||
create_test_global_config "" "" | ||
|
||
TEST_NAME="${TEST_NAME_BASE}" | ||
install_workflow "${TEST_NAME}" "${TEST_NAME_BASE}" | ||
|
||
# Validate the config | ||
run_ok "${TEST_NAME}-validate" cylc validate "${WORKFLOW_NAME}" | ||
|
||
# Run the workflow | ||
workflow_run_ok "${TEST_NAME_BASE}-run" cylc play "${WORKFLOW_NAME}" --no-detach -v | ||
|
||
# Reload happened | ||
grep_ok "Reloading the global configuration" "${WORKFLOW_RUN_DIR}/log/scheduler/01-start-01.log" | ||
|
||
# Reload hasn't happened in job a, has happened in b and c | ||
grep_fail "global init-script reloaded!" "${WORKFLOW_RUN_DIR}/log/job/1/a/01/job.out" | ||
grep_ok "global init-script reloaded!" "${WORKFLOW_RUN_DIR}/log/job/1/b/01/job.out" | ||
grep_ok "global init-script reloaded!" "${WORKFLOW_RUN_DIR}/log/job/1/c/01/job.out" | ||
|
||
# Events are original in job a, updated in b and c | ||
grep_fail "!!EVENT!! succeeded 1/a" "${WORKFLOW_RUN_DIR}/log/scheduler/01-start-01.log" | ||
grep_ok "!!EVENT!! succeeded 1/b" "${WORKFLOW_RUN_DIR}/log/scheduler/01-start-01.log" | ||
grep_ok "!!EVENT!! succeeded 1/c" "${WORKFLOW_RUN_DIR}/log/scheduler/01-start-01.log" | ||
|
||
purge | ||
exit |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
[scheduling] | ||
cycling mode = integer | ||
[[graph]] | ||
R1 = a => reload-global => b & c | ||
[runtime] | ||
[[root]] | ||
script = true | ||
[[a,b]] | ||
platform = localhost | ||
[[c]] | ||
platform = $CYLC_TEST_PLATFORM | ||
[[reload-global]] | ||
platform = localhost | ||
script = """ | ||
# Append to global config | ||
cat >> "$CYLC_CONF_PATH/global.cylc" <<EOF | ||
[platforms] | ||
[[localhost,${CYLC_TEST_PLATFORM}]] | ||
global init-script = echo "global init-script reloaded!" | ||
[task events] | ||
handlers = echo "!!EVENT!! %(event)s %(id)s" | ||
handler events = succeeded | ||
EOF | ||
|
||
# Log the new config | ||
cylc config | ||
|
||
# Reload the global config | ||
cylc reload --global "${CYLC_WORKFLOW_ID}" | ||
""" |
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.