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.

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

use regex to find variable tokens #239

Merged
merged 2 commits into from
Jul 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion merlin/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}."
Expand Down
12 changes: 8 additions & 4 deletions merlin/spec/expansion.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"}
Expand Down Expand Up @@ -74,7 +78,7 @@ def var_ref(string):
by $(<str>).
"""
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})"
Expand All @@ -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))
Expand Down Expand Up @@ -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
Expand Down
10 changes: 6 additions & 4 deletions merlin/spec/specification.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
)


Expand Down Expand Up @@ -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."""

Expand All @@ -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"])

Expand Down
3 changes: 2 additions & 1 deletion merlin/study/study.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
Expand Down
19 changes: 19 additions & 0 deletions merlin/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 4 additions & 1 deletion tests/integration/run_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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": (
Expand Down