From 2b6a70592e6f31b4f4c4f1738761074bd1b3f8c7 Mon Sep 17 00:00:00 2001 From: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> Date: Thu, 27 Jun 2024 17:43:51 +0100 Subject: [PATCH 1/4] Tests: fix `contains_ok` UTF-8 issue See https://stackoverflow.com/q/78678882/3217306 --- tests/functional/lib/bash/test_header | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/lib/bash/test_header b/tests/functional/lib/bash/test_header index 915821c0889..cc0f6e0af43 100644 --- a/tests/functional/lib/bash/test_header +++ b/tests/functional/lib/bash/test_header @@ -386,7 +386,7 @@ contains_ok() { local FILE_CONTROL="${2:--}" local TEST_NAME TEST_NAME="$(basename "${FILE_TEST}")-contains-ok" - comm -13 <(sort "${FILE_TEST}") <(sort "${FILE_CONTROL}") \ + LANG=C comm -13 <(sort "${FILE_TEST}") <(sort "${FILE_CONTROL}") \ 1>"${TEST_NAME}.stdout" 2>"${TEST_NAME}.stderr" if [[ -s "${TEST_NAME}.stdout" ]]; then mkdir -p "${TEST_LOG_DIR}" From 74a77ac1c94cf83c08ab95b88518e0670503641c Mon Sep 17 00:00:00 2001 From: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> Date: Wed, 19 Jun 2024 13:25:02 +0100 Subject: [PATCH 2/4] `cylc stop --now`: fix flaky test & clarify docs --- cylc/flow/scripts/stop.py | 6 +++--- tests/functional/shutdown/08-now1.t | 6 +----- tests/functional/shutdown/08-now1/flow.cylc | 3 +-- 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/cylc/flow/scripts/stop.py b/cylc/flow/scripts/stop.py index 543bb257dea..d49a47aea1e 100755 --- a/cylc/flow/scripts/stop.py +++ b/cylc/flow/scripts/stop.py @@ -53,9 +53,9 @@ CCYYMMDDThh:mm, CCYY-MM-DDThh, etc). Tasks that become ready after the shutdown is ordered will be submitted -immediately if the workflow is restarted. Remaining task event handlers and -job poll and kill commands, however, will be executed prior to shutdown, unless ---now is used. +immediately if the workflow is restarted. Remaining task event handlers and +job poll and kill commands will be executed prior to shutdown, unless +--now is used twice. This command exits immediately unless --max-polls is greater than zero, in which case it polls to wait for workflow shutdown. diff --git a/tests/functional/shutdown/08-now1.t b/tests/functional/shutdown/08-now1.t index 77a822f5cc2..01a4eafdeb5 100755 --- a/tests/functional/shutdown/08-now1.t +++ b/tests/functional/shutdown/08-now1.t @@ -18,7 +18,7 @@ # Test "cylc stop --now" will wait for event handler. . "$(dirname "$0")/test_header" -set_test_number 6 +set_test_number 5 install_workflow "${TEST_NAME_BASE}" "${TEST_NAME_BASE}" run_ok "${TEST_NAME_BASE}-validate" cylc validate "${WORKFLOW_NAME}" @@ -27,10 +27,6 @@ LOGD="$RUN_DIR/${WORKFLOW_NAME}/log" grep_ok 'INFO - Workflow shutting down - REQUEST(NOW)' "${LOGD}/scheduler/log" JLOGD="${LOGD}/job/1/t1/01" # Check that 1/t1 event handler runs -run_ok "${TEST_NAME_BASE}-activity-log-succeeded" \ - grep -q -F \ - "[(('event-handler-00', 'succeeded'), 1) out] Well done 1/t1 succeeded" \ - "${JLOGD}/job-activity.log" run_ok "${TEST_NAME_BASE}-activity-log-started" \ grep -q -F \ "[(('event-handler-00', 'started'), 1) out] Hello 1/t1 started" \ diff --git a/tests/functional/shutdown/08-now1/flow.cylc b/tests/functional/shutdown/08-now1/flow.cylc index a37eea505e2..793d4427ca4 100644 --- a/tests/functional/shutdown/08-now1/flow.cylc +++ b/tests/functional/shutdown/08-now1/flow.cylc @@ -3,7 +3,7 @@ abort on stall timeout = True stall timeout = PT0S abort on inactivity timeout = True - inactivity timeout = PT3M + inactivity timeout = PT1M [scheduling] [[graph]] @@ -14,6 +14,5 @@ script = cylc__job__wait_cylc_message_started; cylc stop --now "${CYLC_WORKFLOW_ID}" [[[events]]] started handlers = sleep 10 && echo 'Hello %(id)s %(event)s' - succeeded handlers = echo 'Well done %(id)s %(event)s' [[t2]] script = true From cd18952ce52f77f294e76633f363c0a91b029951 Mon Sep 17 00:00:00 2001 From: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> Date: Mon, 1 Jul 2024 12:22:36 +0100 Subject: [PATCH 3/4] flake8-comprehensions --- cylc/flow/network/schema.py | 2 +- cylc/flow/task_outputs.py | 6 +----- cylc/flow/tui/updater.py | 7 ++----- 3 files changed, 4 insertions(+), 11 deletions(-) diff --git a/cylc/flow/network/schema.py b/cylc/flow/network/schema.py index 3a8626e2c95..70e40232c1d 100644 --- a/cylc/flow/network/schema.py +++ b/cylc/flow/network/schema.py @@ -502,7 +502,7 @@ async def get_nodes_edges(root, info: 'ResolveInfo', **args): def resolve_state_totals(root, info, **args): - state_totals = {state: 0 for state in TASK_STATUSES_ORDERED} + state_totals = dict.fromkeys(TASK_STATUSES_ORDERED, 0) # Update with converted protobuf map container state_totals.update( dict(getattr(root, to_snake_case(info.field_name), {}))) diff --git a/cylc/flow/task_outputs.py b/cylc/flow/task_outputs.py index 40b9d991b30..1af37e1554e 100644 --- a/cylc/flow/task_outputs.py +++ b/cylc/flow/task_outputs.py @@ -251,11 +251,7 @@ def get_optional_outputs( ) for output in used_compvars }, - # the outputs that are not used in the expression - **{ - output: None - for output in all_compvars - used_compvars - }, + **dict.fromkeys(all_compvars - used_compvars), } diff --git a/cylc/flow/tui/updater.py b/cylc/flow/tui/updater.py index 26a8614b1a3..2a28b5d7906 100644 --- a/cylc/flow/tui/updater.py +++ b/cylc/flow/tui/updater.py @@ -68,11 +68,8 @@ def get_default_filters(): These filters show everything. """ return { - 'tasks': { - # filtered task statuses - state: True - for state in TASK_STATUSES_ORDERED - }, + # filtered task statuses + 'tasks': dict.fromkeys(TASK_STATUSES_ORDERED, True), 'workflows': { # filtered workflow statuses **{ From 6d311118ebe3c41bdb6a2a5e95167264f8db7a45 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Tue, 2 Jul 2024 10:47:01 +0100 Subject: [PATCH 4/4] commands: log command validation errors (#6164) --- cylc/flow/network/resolvers.py | 1 + tests/integration/test_resolvers.py | 56 +++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/cylc/flow/network/resolvers.py b/cylc/flow/network/resolvers.py index 65983042532..11eafd8bea5 100644 --- a/cylc/flow/network/resolvers.py +++ b/cylc/flow/network/resolvers.py @@ -761,6 +761,7 @@ async def _mutation_mapper( except Exception as exc: # NOTE: keep this exception vague to prevent a bad command taking # down the scheduler + LOG.warning(f'{log1}\n{exc.__class__.__name__}: {exc}') if cylc.flow.flags.verbosity > 1: LOG.exception(exc) # log full traceback in debug mode return (False, str(exc)) diff --git a/tests/integration/test_resolvers.py b/tests/integration/test_resolvers.py index cfdc8cafc3d..981237a4d2a 100644 --- a/tests/integration/test_resolvers.py +++ b/tests/integration/test_resolvers.py @@ -250,3 +250,59 @@ async def test_command_logging(mock_flow, caplog, log_filter): await mock_flow.resolvers._mutation_mapper("put_messages", kwargs, meta) assert log_filter( caplog, contains='Command "put_messages" received from Dr Spock') + + +async def test_command_validation_failure( + mock_flow, + caplog, + flow_args, + monkeypatch, +): + """It should log command validation failures server side.""" + caplog.set_level(logging.DEBUG, None) + flow_args['workflows'].append( + { + 'user': mock_flow.owner, + 'workflow': mock_flow.name, + 'workflow_sel': None, + } + ) + + # submit a command with invalid arguments: + async def submit_invalid_command(verbosity=0): + nonlocal caplog, mock_flow, flow_args + monkeypatch.setattr('cylc.flow.flags.verbosity', verbosity) + caplog.clear() + return await mock_flow.resolvers.mutator( + None, + 'stop', + flow_args, + {'task': 'cycle/task/job', 'mode': 'not-a-mode'}, + {}, + ) + + # submitting the invalid command should result in this error + msg = 'This command does not take job ids:\n * cycle/task/job' + + # test submitting the command at *default* verbosity + response = await submit_invalid_command() + + # the error should be sent back to the client: + assert response[0]['response'][1] == msg + # it should also be logged by the server: + assert caplog.records[-1].levelno == logging.WARNING + assert msg in caplog.records[-1].message + + # test submitting the command at *debug* verbosity + response = await submit_invalid_command(verbosity=2) + + # the error should be sent back to the client: + assert response[0]['response'][1] == msg + # it should be logged at the server + assert caplog.records[-2].levelno == logging.WARNING + assert msg in caplog.records[-2].message + # the traceback should also be logged + # (note traceback gets logged at the ERROR level and shows up funny in + # caplog) + assert caplog.records[-1].levelno == logging.ERROR + assert msg in caplog.records[-1].message