From fcc642b9cfa3e98ebdaa8c671115d053db53ca21 Mon Sep 17 00:00:00 2001 From: Tom Coleman <15375218+ColemanTom@users.noreply.github.com> Date: Wed, 22 Nov 2023 09:54:14 +1100 Subject: [PATCH 1/8] Add a new entry_point for xtriggers This creates a clean way to add xtriggers from python packages. xtriggers no longer need to be directly in the PYTHONPATH, but instead can be added via a cylc.xtriggers entry point. Local cylc-flow xtriggers are defined via a new entry_point. --- cylc/flow/subprocpool.py | 27 ++++++++++++++++++--------- setup.cfg | 6 ++++++ 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/cylc/flow/subprocpool.py b/cylc/flow/subprocpool.py index c380bdaa3e8..efeb6be42fd 100644 --- a/cylc/flow/subprocpool.py +++ b/cylc/flow/subprocpool.py @@ -28,7 +28,7 @@ from subprocess import DEVNULL, run # nosec from typing import Any, Callable, List, Optional -from cylc.flow import LOG +from cylc.flow import LOG, iter_entry_points from cylc.flow.cfgspec.glbl_cfg import glbl_cfg from cylc.flow.cylc_subproc import procopen from cylc.flow.exceptions import PlatformLookupError @@ -69,29 +69,38 @@ def _killpg(proc, signal): def get_func(func_name, src_dir): """Find and return an xtrigger function from a module of the same name. - Can be in /lib/python, CYLC_MOD_LOC, or in Python path. + Can be in /lib/python, in Python path, or defined via an + entry_point. Loctions checked are in this order. + Workflow source directory passed in as this is executed in an independent process in the command pool and therefore doesn't know about the workflow. """ if func_name in _XTRIG_FUNCS: return _XTRIG_FUNCS[func_name] + # First look in /lib/python. sys.path.insert(0, os.path.join(src_dir, 'lib', 'python')) mod_name = func_name try: mod_by_name = __import__(mod_name, fromlist=[mod_name]) except ImportError: - # Then look in built-in xtriggers. - mod_name = "%s.%s" % ("cylc.flow.xtriggers", func_name) - try: - mod_by_name = __import__(mod_name, fromlist=[mod_name]) - except ImportError: - raise + # Look for xtriggers via entry_points for external sources. + # Do this after the lib/python and PYTHONPATH approaches to allow + # users to override entry_point definitions with local/custom + # implementations. + for entry_point in iter_entry_points('cylc.xtriggers'): + if func_name == entry_point.name: + _XTRIG_FUNCS[func_name] = entry_point.load() + return _XTRIG_FUNCS[func_name] + + # Still unable to find anything so abort + raise + try: _XTRIG_FUNCS[func_name] = getattr(mod_by_name, func_name) except AttributeError: - # Module func_name has no function func_name. + # Module func_name has no function func_name, nor an entry_point entry. raise return _XTRIG_FUNCS[func_name] diff --git a/setup.cfg b/setup.cfg index e8e7f46b2d2..954c194b11d 100644 --- a/setup.cfg +++ b/setup.cfg @@ -215,6 +215,12 @@ cylc.main_loop = cylc.pre_configure = cylc.post_install = log_vc_info = cylc.flow.install_plugins.log_vc_info:main +# NOTE: Built-in xtriggers +cylc.xtriggers = + echo = cylc.flow.xtriggers.echo:echo + wall_clock = cylc.flow.xtriggers.wall_clock:wall_clock + workflow_state = cylc.flow.xtriggers.workflow_state:workflow_state + xrandom = cylc.flow.xtriggers.xrandom:xrandom [bdist_rpm] requires = From cd55e6db88e9ce83c829bd6ceb847fab6438d7b3 Mon Sep 17 00:00:00 2001 From: Tom Coleman <15375218+ColemanTom@users.noreply.github.com> Date: Thu, 23 Nov 2023 10:09:40 +1100 Subject: [PATCH 2/8] Add changes file for PR 5831 --- changes.d/5831.feat.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes.d/5831.feat.md diff --git a/changes.d/5831.feat.md b/changes.d/5831.feat.md new file mode 100644 index 00000000000..daecc5e7a87 --- /dev/null +++ b/changes.d/5831.feat.md @@ -0,0 +1 @@ +Add capability to install xtriggers via a new cylc.xtriggers entry point From be3e27767301b822d2ca63bb3c029b604cea919c Mon Sep 17 00:00:00 2001 From: Tom Coleman <15375218+ColemanTom@users.noreply.github.com> Date: Wed, 6 Dec 2023 14:45:02 +1100 Subject: [PATCH 3/8] Add xtrigger CYLC_PYTHONPATH functional test Ensures that xtrggers in your CYLC_PYTHONPATH are respected above entry_point xtriggers. --- .../xtriggers/04-respect-cylc-pythonpath.t | 47 +++++++++++++++++++ .../04-respect-cylc-pythonpath/dir/echo.py | 21 +++++++++ .../04-respect-cylc-pythonpath/flow.cylc | 15 ++++++ 3 files changed, 83 insertions(+) create mode 100644 tests/functional/xtriggers/04-respect-cylc-pythonpath.t create mode 100644 tests/functional/xtriggers/04-respect-cylc-pythonpath/dir/echo.py create mode 100644 tests/functional/xtriggers/04-respect-cylc-pythonpath/flow.cylc diff --git a/tests/functional/xtriggers/04-respect-cylc-pythonpath.t b/tests/functional/xtriggers/04-respect-cylc-pythonpath.t new file mode 100644 index 00000000000..f35ece93b3e --- /dev/null +++ b/tests/functional/xtriggers/04-respect-cylc-pythonpath.t @@ -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 . +#------------------------------------------------------------------------------ + +# Test persistence of xtrigger results across restart. A cycling task depends +# on a non cycle-point dependent custom xtrigger called "faker". In the first +# cycle point the xtrigger succeeds and returns a result, then a task shuts +# the workflow down. Then we replace the custom xtrigger function with one that +# will fail if called again - which should not happen because the original +# result should be remembered (as this xtrigger is not cycle point dependent). +# Also test the correct result is broadcast to the dependent task before and +# after workflow restart. + +. "$(dirname "$0")/test_header" +set_test_number 3 + +install_workflow "${TEST_NAME_BASE}" "${TEST_NAME_BASE}" + +# Install the succeeding xtrigger function. +export CYLC_PYTHONPATH=${WORKFLOW_RUN_DIR}/dir:${CYLC_PYTHONPATH:-} + +# Validate the test workflow. +run_ok "${TEST_NAME_BASE}-val" cylc validate --debug "${WORKFLOW_NAME}" + +# Run the first cycle, till auto shutdown by task. +TEST_NAME="${TEST_NAME_BASE}-run" +workflow_run_ok "${TEST_NAME}" cylc play --no-detach --debug "${WORKFLOW_NAME}" + +# Check the broadcast result of xtrigger. +cylc cat-log "${WORKFLOW_NAME}" >'scheduler.log.out' +grep_ok "echo overridden, args=('the_args',)" 'scheduler.log.out' + +purge diff --git a/tests/functional/xtriggers/04-respect-cylc-pythonpath/dir/echo.py b/tests/functional/xtriggers/04-respect-cylc-pythonpath/dir/echo.py new file mode 100644 index 00000000000..1c38f3ff920 --- /dev/null +++ b/tests/functional/xtriggers/04-respect-cylc-pythonpath/dir/echo.py @@ -0,0 +1,21 @@ +#!/usr/bin/env python3 +# 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 . + + +def echo(*args, **kwargs): + print(f"echo overridden, args={args}") + return (True, {}) diff --git a/tests/functional/xtriggers/04-respect-cylc-pythonpath/flow.cylc b/tests/functional/xtriggers/04-respect-cylc-pythonpath/flow.cylc new file mode 100644 index 00000000000..1eb552d84ac --- /dev/null +++ b/tests/functional/xtriggers/04-respect-cylc-pythonpath/flow.cylc @@ -0,0 +1,15 @@ +[scheduler] + cycle point format = %Y +[scheduling] + initial cycle point = 2010 + final cycle point = 2011 + [[xtriggers]] + x1 = echo("the_args") + [[graph]] + R1 = "@x1 => foo => shutdown" + P1Y = "@x1 => foo" +[runtime] + [[foo]] + script = true + [[shutdown]] + script = "cylc shutdown $CYLC_WORKFLOW_ID" From 78f695fc2395e9395b93b6efb1e27e8fb530cd3c Mon Sep 17 00:00:00 2001 From: ColemanTom <15375218+ColemanTom@users.noreply.github.com> Date: Thu, 14 Dec 2023 08:06:04 +1100 Subject: [PATCH 4/8] Update tests/functional/xtriggers/04-respect-cylc-pythonpath.t Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> --- tests/functional/xtriggers/04-respect-cylc-pythonpath.t | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/functional/xtriggers/04-respect-cylc-pythonpath.t b/tests/functional/xtriggers/04-respect-cylc-pythonpath.t index f35ece93b3e..af7b6524f62 100644 --- a/tests/functional/xtriggers/04-respect-cylc-pythonpath.t +++ b/tests/functional/xtriggers/04-respect-cylc-pythonpath.t @@ -36,7 +36,6 @@ export CYLC_PYTHONPATH=${WORKFLOW_RUN_DIR}/dir:${CYLC_PYTHONPATH:-} # Validate the test workflow. run_ok "${TEST_NAME_BASE}-val" cylc validate --debug "${WORKFLOW_NAME}" -# Run the first cycle, till auto shutdown by task. TEST_NAME="${TEST_NAME_BASE}-run" workflow_run_ok "${TEST_NAME}" cylc play --no-detach --debug "${WORKFLOW_NAME}" From 17a5266d3823e5e4c1a8f8747df0e9bd55c499eb Mon Sep 17 00:00:00 2001 From: ColemanTom <15375218+ColemanTom@users.noreply.github.com> Date: Thu, 14 Dec 2023 08:06:51 +1100 Subject: [PATCH 5/8] Simplify 04-respect-cylc-pythonpath flow.cylc test file Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> --- .../xtriggers/04-respect-cylc-pythonpath/flow.cylc | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/tests/functional/xtriggers/04-respect-cylc-pythonpath/flow.cylc b/tests/functional/xtriggers/04-respect-cylc-pythonpath/flow.cylc index 1eb552d84ac..da05eb9e5ce 100644 --- a/tests/functional/xtriggers/04-respect-cylc-pythonpath/flow.cylc +++ b/tests/functional/xtriggers/04-respect-cylc-pythonpath/flow.cylc @@ -1,15 +1,8 @@ -[scheduler] - cycle point format = %Y [scheduling] - initial cycle point = 2010 - final cycle point = 2011 [[xtriggers]] x1 = echo("the_args") [[graph]] - R1 = "@x1 => foo => shutdown" - P1Y = "@x1 => foo" + R1 = @x1 => foo [runtime] [[foo]] script = true - [[shutdown]] - script = "cylc shutdown $CYLC_WORKFLOW_ID" From 658115346d02fc8f81bef95879962d2f08a84b5a Mon Sep 17 00:00:00 2001 From: ColemanTom <15375218+ColemanTom@users.noreply.github.com> Date: Thu, 14 Dec 2023 08:07:32 +1100 Subject: [PATCH 6/8] Update description of 04-respect-cylc-pythonpath.t Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> --- .../functional/xtriggers/04-respect-cylc-pythonpath.t | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/tests/functional/xtriggers/04-respect-cylc-pythonpath.t b/tests/functional/xtriggers/04-respect-cylc-pythonpath.t index af7b6524f62..a4a657e7085 100644 --- a/tests/functional/xtriggers/04-respect-cylc-pythonpath.t +++ b/tests/functional/xtriggers/04-respect-cylc-pythonpath.t @@ -16,14 +16,8 @@ # along with this program. If not, see . #------------------------------------------------------------------------------ -# Test persistence of xtrigger results across restart. A cycling task depends -# on a non cycle-point dependent custom xtrigger called "faker". In the first -# cycle point the xtrigger succeeds and returns a result, then a task shuts -# the workflow down. Then we replace the custom xtrigger function with one that -# will fail if called again - which should not happen because the original -# result should be remembered (as this xtrigger is not cycle point dependent). -# Also test the correct result is broadcast to the dependent task before and -# after workflow restart. +# An xtrigger added to $CYLC_PYTHONPATH should take precedence +# over a `cylc.xtriggers` entry point of the same name . "$(dirname "$0")/test_header" set_test_number 3 From badcdb1b1e9e464763650fbaeaa5583459d6b9ae Mon Sep 17 00:00:00 2001 From: ColemanTom <15375218+ColemanTom@users.noreply.github.com> Date: Thu, 14 Dec 2023 08:07:54 +1100 Subject: [PATCH 7/8] Simplify final test in xtriggers/04-respect-cylc-pythonpath.t Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> --- tests/functional/xtriggers/04-respect-cylc-pythonpath.t | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/functional/xtriggers/04-respect-cylc-pythonpath.t b/tests/functional/xtriggers/04-respect-cylc-pythonpath.t index a4a657e7085..5cc05cfcb89 100644 --- a/tests/functional/xtriggers/04-respect-cylc-pythonpath.t +++ b/tests/functional/xtriggers/04-respect-cylc-pythonpath.t @@ -33,8 +33,7 @@ run_ok "${TEST_NAME_BASE}-val" cylc validate --debug "${WORKFLOW_NAME}" TEST_NAME="${TEST_NAME_BASE}-run" workflow_run_ok "${TEST_NAME}" cylc play --no-detach --debug "${WORKFLOW_NAME}" -# Check the broadcast result of xtrigger. -cylc cat-log "${WORKFLOW_NAME}" >'scheduler.log.out' -grep_ok "echo overridden, args=('the_args',)" 'scheduler.log.out' +# Check the result of xtrigger. +grep_workflow_log_ok "${TEST_NAME_BASE}-grep" "echo overridden, args=('the_args',)" purge From c08996f518e55275371f48c86474db1b1b533694 Mon Sep 17 00:00:00 2001 From: ColemanTom <15375218+ColemanTom@users.noreply.github.com> Date: Thu, 14 Dec 2023 08:08:25 +1100 Subject: [PATCH 8/8] Improve docstring for xtrigger location order specification Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> --- cylc/flow/subprocpool.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/cylc/flow/subprocpool.py b/cylc/flow/subprocpool.py index efeb6be42fd..9ac3845bb54 100644 --- a/cylc/flow/subprocpool.py +++ b/cylc/flow/subprocpool.py @@ -69,8 +69,11 @@ def _killpg(proc, signal): def get_func(func_name, src_dir): """Find and return an xtrigger function from a module of the same name. - Can be in /lib/python, in Python path, or defined via an - entry_point. Loctions checked are in this order. + These locations are checked in this order: + - /lib/python/ + - `$CYLC_PYTHONPATH` + - defined via a `cylc.xtriggers` entry point for an + installed Python package. Workflow source directory passed in as this is executed in an independent process in the command pool and therefore doesn't know about the workflow.