Skip to content
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.

Sign up for GitHub

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

Fix suite freeze on non-existent xtrigger - 7.8.x #3059

Merged
merged 2 commits into from
Apr 3, 2019
Merged

Fix suite freeze on non-existent xtrigger - 7.8.x #3059

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
212606b
initial port of pull/3056
dwsutherland Apr 1, 2019
2a1eca3
style fix
dwsutherland Apr 2, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions lib/cylc/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
from cylc.graphnode import GraphNodeParser, GraphNodeError
from cylc.print_tree import print_tree
from cylc.subprocctx import SubFuncContext
from cylc.subprocpool import get_func
from cylc.suite_srv_files_mgr import SuiteSrvFilesManager
from cylc.taskdef import TaskDef, TaskDefError
from cylc.task_id import TaskID
Expand Down Expand Up @@ -2100,6 +2101,15 @@ def load_graph(self):
if offset > old_offset:
self.taskdefs[task_name].xclock_label = label
else:
try:
if not callable(get_func(xtrig.func_name, self.fdir)):
raise SuiteConfigError("ERROR, xtrigger function "
"not callable: "
"%s" % xtrig.func_name)
except ImportError:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, just realized you have ModuleNotFoundError on master, and the parent class ImportError on 7.8.x. I think we better use the parent ImportError on master too @dwsutherland . WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kinow - Yes, as python3 gave me ModuleNotFoundError, but python2 produced ImportError and the former is not recognised. I didn't try ImportError in python3 (Is it back compat?), and figured both have diverged anyway.. Is this ok?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting (another hole in my Python 3 knowledge!).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe ImportError is the parent class of ModuleNotFoundError.

Interesting (another hole in my Python 3 knowledge!).

Me too! I thought at least this part was consistent in both. But catching the parent class should be fine, though better to confirm if that works. Perhaps the unit tests could be used to validate that it works?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe ImportError is the parent class of ModuleNotFoundError.

Well I guess the child doesn't exist in python2 ... Consistency aside, would it have been better to use the parent?

Also does consistency matter between 7.8.x and 8? at some point we'll stop back porting changes? (as I guess it matters for that)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. If that works as-is, all good then I think

raise SuiteConfigError("ERROR, xtrigger function "
"not found: "
"%s" % xtrig.func_name)
self.xtrigger_mgr.add_trig(label, xtrig)
self.taskdefs[task_name].xtrig_labels.add(label)

Expand Down
28 changes: 14 additions & 14 deletions lib/cylc/scheduler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,20 @@ def configure(self):
self.configure_contact()
n_restart = 0

# Copy local python modules from source to run directory
for sub_dir in ["python", os.path.join("lib", "python")]:
# TODO - eventually drop the deprecated "python" sub-dir.
suite_py = os.path.join(self.suite_dir, sub_dir)
if (os.path.realpath(self.suite_dir) !=
os.path.realpath(self.suite_run_dir) and
os.path.isdir(suite_py)):
suite_run_py = os.path.join(self.suite_run_dir, sub_dir)
try:
rmtree(suite_run_py)
except OSError:
pass
copytree(suite_py, suite_run_py)

self.profiler.log_memory("scheduler.py: before load_suiterc")
self.load_suiterc()
self.profiler.log_memory("scheduler.py: after load_suiterc")
Expand Down Expand Up @@ -456,20 +470,6 @@ def configure(self):
self.suite_db_mgr.put_suite_template_vars(self.template_vars)
self.suite_db_mgr.put_runtime_inheritance(self.config)

# Copy local python modules from source to run directory
for sub_dir in ["python", os.path.join("lib", "python")]:
# TODO - eventually drop the deprecated "python" sub-dir.
suite_py = os.path.join(self.suite_dir, sub_dir)
if (os.path.realpath(self.suite_dir) !=
os.path.realpath(self.suite_run_dir) and
os.path.isdir(suite_py)):
suite_run_py = os.path.join(self.suite_run_dir, sub_dir)
try:
rmtree(suite_run_py)
except OSError:
pass
copytree(suite_py, suite_run_py)

self.already_timed_out = False
self.set_suite_timer()

Expand Down
6 changes: 3 additions & 3 deletions lib/cylc/xtrigger_mgr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ class XtriggerManager(object):
'''

Task proxies only store xtriggers labels: clock_0, suite_x, etc. above.
These mapped to the defined function calls. Dependence on xtriggers is
satisfied by calling these functions asynchronously in the task pool
These are mapped to the defined function calls. Dependence on xtriggers
is satisfied by calling these functions asynchronously in the task pool
(except clock triggers which are called synchronously as they're quick).

A unique call is defined by a unique function call signature, i.e. the
Expand Down Expand Up @@ -261,7 +261,7 @@ def callback(self, ctx):
self.active.remove(sig)
try:
satisfied, results = json.loads(ctx.out)
except ValueError:
except (ValueError, TypeError):
return
LOG.debug('%s: returned %s' % (sig, results))
if satisfied:
Expand Down
5 changes: 5 additions & 0 deletions tests/validate/04-builtin-suites.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ for suite in ${SUITES[@]}; do
skip 2 "${TEST_NAME}: EmPy not installed"
continue
fi
# ignore kafka suites TODO: revisit it later (kafka dependency)
if [[ $suite == *"kafka"* ]]; then
skip 2 "${TEST_NAME}: Skipping suites using Kafka dependencies"
continue
fi
run_ok "${TEST_NAME}" cylc validate "${suite}" -v
filter_warnings "${TEST_NAME}.stderr"
cmp_ok "${TEST_NAME}.stderr.processed" /dev/null
Expand Down