Skip to content

Commit

Permalink
Correction to safe pathing for missed cases and make_safe_path enhanc…
Browse files Browse the repository at this point in the history
…ements. (#157)

* Made pickle and log path string safe for pathing.

* Tweaks to make_safe_path to include a base path.

* Updates to make_safe_path usage

* Correction to not modify the iterator copy.
  • Loading branch information
FrankD412 authored and Francesco Di Natale committed Jul 14, 2019
1 parent 681bfcc commit 17a10a1
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 17 deletions.
13 changes: 7 additions & 6 deletions maestrowf/datastructures/core/study.py
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ def _stage_parameterized(self, dag):
# Copy the step and set to not modified.
self.step_combos[step].add(step)

workspace = make_safe_path(self._out_path, step)
workspace = make_safe_path(self._out_path, *[step])
self.workspaces[step] = workspace
logger.debug("Workspace: %s", workspace)

Expand All @@ -527,7 +527,7 @@ def _stage_parameterized(self, dag):
# If we're looking at a parameter independent match
# the workspace is the folder that contains all of
# the outputs of all combinations for the step.
ws = make_safe_path(self._out_path, match)
ws = make_safe_path(self._out_path, *[match])
logger.info("Found funnel workspace -- %s", ws)
else:
ws = self.workspaces[match]
Expand Down Expand Up @@ -584,10 +584,11 @@ def _stage_parameterized(self, dag):
combo_str = combo.get_param_string(self.used_params[step])
if self._hash_ws:
workspace = make_safe_path(
self._out_path, step, md5(combo_str).hexdigest())
self._out_path,
*[step, md5(combo_str).hexdigest()])
else:
workspace = \
make_safe_path(self._out_path, step, combo_str)
make_safe_path(self._out_path, *[step, combo_str])
logger.debug("Workspace: %s", workspace)
combo_str = "{}_{}".format(step, combo_str)
self.workspaces[combo_str] = workspace
Expand All @@ -613,7 +614,7 @@ def _stage_parameterized(self, dag):
# If we're looking at a parameter independent match
# the workspace is the folder that contains all of
# the outputs of all combinations for the step.
ws = make_safe_path(self._out_path, match)
ws = make_safe_path(self._out_path, *[match])
logger.info("Found funnel workspace -- %s", ws)
elif not self.used_params[match]:
# If it's not a funneled dependency and the match
Expand Down Expand Up @@ -695,7 +696,7 @@ def _stage_linear(self, dag):
continue

# Initialize management structures.
ws = make_safe_path(self._out_path, step)
ws = make_safe_path(self._out_path, *[step])
self.workspaces[step] = ws
self.depends[step] = set()
# Hub dependencies are not possible in linear studies. Empty set
Expand Down
20 changes: 11 additions & 9 deletions maestrowf/maestro.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ def run_study(args):
out_dir = environment.remove("OUTPUT_PATH")
if args.out:
# If out is specified in the args, ignore OUTPUT_PATH.
output_path = os.path.abspath(make_safe_path(args.out))
output_path = os.path.abspath(args.out)

# If we are automatically launching, just set the input as yes.
if os.path.exists(output_path):
Expand Down Expand Up @@ -167,7 +167,7 @@ def run_study(args):
spec.name.replace(" ", "_"),
time.strftime("%Y%m%d-%H%M%S")
)
output_path = make_safe_path(out_dir, out_name)
output_path = make_safe_path(out_dir, *[out_name])
environment.add(Variable("OUTPUT_PATH", output_path))

# Now that we know outpath, set up logging.
Expand Down Expand Up @@ -236,8 +236,7 @@ def run_study(args):
raise NotImplementedError("The 'dryrun' mode is in development.")

# Pickle up the DAG
pkl_path = os.path.join(path, "{}.pkl".format(
study.name.replace(" ", "_").lower()))
pkl_path = make_safe_path(path, *["{}.pkl".format(study.name)])
exec_dag.pickle(pkl_path)

# If we are automatically launching, just set the input as yes.
Expand All @@ -259,12 +258,15 @@ def run_study(args):
return completion_status.value
else:
# Launch manager with nohup
log_path = make_safe_path(
study.output_path,
*["{}.txt".format(exec_dag.name)])

cmd = ["nohup", "conductor",
"-t", str(args.sleeptime),
"-d", str(args.debug_lvl),
path,
"&>", "{}.txt".format(os.path.join(
study.output_path, exec_dag.name))]
"&>", log_path]
LOGGER.debug(" ".join(cmd))
start_process(" ".join(cmd))

Expand Down Expand Up @@ -385,7 +387,7 @@ def setup_logging(args, path, name):
logpath = args.logpath
# Otherwise, we should just output to the OUTPUT_PATH.
else:
logpath = make_safe_path(path, "logs")
logpath = make_safe_path(path, *["logs"])

loglevel = args.debug_lvl * 10

Expand All @@ -394,8 +396,8 @@ def setup_logging(args, path, name):
formatter = logging.Formatter(LFORMAT)
ROOTLOGGER.setLevel(loglevel)

logname = make_safe_path("{}.log".format(name))
fh = logging.FileHandler(os.path.join(logpath, logname))
log_path = make_safe_path(logpath, *["{}.log".format(name)])
fh = logging.FileHandler(log_path)
fh.setLevel(loglevel)
fh.setFormatter(formatter)
ROOTLOGGER.addHandler(fh)
Expand Down
13 changes: 11 additions & 2 deletions maestrowf/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,12 +150,21 @@ def csvtable_to_dict(fstream):
return table


def make_safe_path(*args):
def make_safe_path(base_path, *args):
"""
Construct a subpath that is path safe.
:params base_path: The base path to append args to.
:params *args: Path components to join into a path.
:returns: A joined subpath with invalid characters stripped.
"""
valid = "-_.() {}{}".format(string.ascii_letters, string.digits)
path = [base_path]
for arg in args:
arg = "".join(c for c in arg if c in valid)
arg = arg.replace(" ", "_")
return os.path.join(*args)
path.append(arg)
return os.path.join(*path)


def start_process(cmd, cwd=None, env=None, shell=True):
Expand Down

0 comments on commit 17a10a1

Please sign in to comment.