diff --git a/merlin/main.py b/merlin/main.py index 8b23b7f08..2151eed46 100644 --- a/merlin/main.py +++ b/merlin/main.py @@ -127,7 +127,9 @@ def parse_override_vars(variables_list): ) key = entry[0] if key is None or key == "" or "$" in key: - raise ValueError("--vars requires valid variable names.") + raise ValueError( + "--vars requires valid variable names comprised of alphanumeric characters and underscores." + ) if key in RESERVED: raise ValueError( f"Cannot override reserved word '{key}'! Reserved words are: {RESERVED}." diff --git a/merlin/spec/expansion.py b/merlin/spec/expansion.py index ef5999571..eadd21c65 100644 --- a/merlin/spec/expansion.py +++ b/merlin/spec/expansion.py @@ -41,6 +41,10 @@ error_override_vars, ) from merlin.spec.specification import MerlinSpec +from merlin.utils import ( + contains_shell_ref, + contains_token, +) MAESTRO_RESERVED = {"SPECROOT", "WORKSPACE", "LAUNCHER"} @@ -74,7 +78,7 @@ def var_ref(string): by $(). """ string = string.upper() - if "$(" in string: + if contains_token(string): LOG.warning(f"Bad var_ref usage on string '{string}'.") return string return f"$({string})" @@ -86,7 +90,7 @@ def expand_line(line, var_dict): and user variables, as well as variables in 'var_dict'. """ line = expandvars(expanduser(line)) - if "$" not in line: + if not contains_token(line): return line for key, val in var_dict.items(): line = line.replace(var_ref(key), str(val)) @@ -133,14 +137,14 @@ def determine_user_variables(*user_var_dicts): f"Cannot reassign value of reserved word '{key}'! Reserved words are: {RESERVED}." ) new_val = str(val) - if "$(" in new_val: # change to re + if contains_token(new_val): for determined_key in determined_results.keys(): var_determined_key = var_ref(determined_key) if var_determined_key in new_val: new_val = new_val.replace( var_determined_key, determined_results[determined_key] ) - if "$" in new_val: # change to re + if contains_shell_ref(new_val): new_val = expandvars(new_val) determined_results[key.upper()] = new_val return determined_results diff --git a/merlin/spec/specification.py b/merlin/spec/specification.py index 1275b5572..9221d2af6 100644 --- a/merlin/spec/specification.py +++ b/merlin/spec/specification.py @@ -36,17 +36,17 @@ import json import logging import os -from io import StringIO from contextlib import suppress +from io import StringIO import jsonschema import yaml from maestrowf.specification.yamlspecification import YAMLSpecification from merlin.spec import ( + SCHEMA_PATH, all_keys, defaults, - SCHEMA_PATH ) @@ -222,7 +222,7 @@ def get_worker_names(self): for worker in self.merlin["resources"]["workers"]: result.append(worker) return result - + def verify(self): """Validate the specification.""" @@ -237,7 +237,9 @@ def verify(self): super().validate_schema("env", self.environment, schemas["ENV"]) super().validate_schema("batch", self.batch, schemas["BATCH"]) for step in self.study: - super().validate_schema(f"study step {step['name']}", step, schemas["STUDY_STEP"]) + super().validate_schema( + f"study step {step['name']}", step, schemas["STUDY_STEP"] + ) for param, contents in self.globals.items(): super().validate_schema("global.params", contents, schemas["PARAM"]) diff --git a/merlin/study/study.py b/merlin/study/study.py index f73577985..ade49cb7f 100644 --- a/merlin/study/study.py +++ b/merlin/study/study.py @@ -52,6 +52,7 @@ from merlin.spec.specification import MerlinSpec from merlin.study.dag import DAG from merlin.utils import ( + contains_token, get_flux_cmd, load_array_file, ) @@ -353,7 +354,7 @@ def expanded_spec(self): ) # expand provenance spec filename - if "$(" in self.spec.name: + if contains_token(self.spec.name): expanded_name = result.description["name"].replace(" ", "_") + ".yaml" expanded_workspace = os.path.join( self.output_path, diff --git a/merlin/utils.py b/merlin/utils.py index 62c7f3956..54d9cb526 100644 --- a/merlin/utils.py +++ b/merlin/utils.py @@ -439,3 +439,22 @@ def check_machines(machines): return True return False + + +def contains_token(string): + """ + Return True if given string contains a token of the form $(STR). + """ + if re.search(r"\$\(\w+\)", string): + return True + return False + + +def contains_shell_ref(string): + """ + Return True if given string contains a shell variable reference + of the form $STR or ${STR}. + """ + if re.search(r"\$\w+", string) or re.search(r"\$\{\w+\}", string): + return True + return False diff --git a/tests/integration/run_tests.py b/tests/integration/run_tests.py index 671795439..ccd4cbef5 100644 --- a/tests/integration/run_tests.py +++ b/tests/integration/run_tests.py @@ -411,7 +411,10 @@ def define_tests(): ), "local format 0": ( f"merlin -lvl debug run {dev_examples}/full_format.yaml --local", - [RegexCond("Spec verified. No errors found."), RegexCond("Merlin block verified. No errors found.")], + [ + RegexCond("Spec verified. No errors found."), + RegexCond("Merlin block verified. No errors found."), + ], "local", ), "local format 1": (