Repository: pylhc/submitter

Issue:
add masked commandline option
Currently, only a mask file can be used for submission.
Alternative option could be to allow a masked commandline input like `--commandparam1=%(param1)s  --commandparam2=%(param2)s` .

Discussion:
mihofer: before we go, bump to 1.0.0?
means also changelog should be fixed ..
fsoubelet: > before we go, bump to 1.0.0?

I would say no as I'm attached to `SemVer`.


Patch:
diff --git a/README.md b/README.md
index e9f7ac3..fef7ec4 100644
--- a/README.md
+++ b/README.md
@@ -11,7 +11,7 @@ pylhc-submitter contains scripts to simplify the creation and submission of jobs
 ## Documentation
 
 - Autogenerated docs via `Sphinx` can be found at <https://pylhc.github.io/submitter/>.
-- General documentation of the OMC-Team is located at <https://pylhc.github.io/>
+- General documentation of the OMC-Team, including examples, is located at <https://pylhc.github.io/>
 
 ## Getting Started
 
diff --git a/doc/entrypoints/submitter.rst b/doc/entrypoints/submitter.rst
index 242fe9a..c22025b 100644
--- a/doc/entrypoints/submitter.rst
+++ b/doc/entrypoints/submitter.rst
@@ -1,9 +1,9 @@
 Job Submitter (for HTCondor)
 ****************************
 
-.. automodule:: pylhc.job_submitter
+.. automodule:: pylhc_submitter.job_submitter
     :members:
 
 
-.. automodule:: pylhc.autosix
+.. automodule:: pylhc_submitter.autosix
     :members:
diff --git a/doc/modules/constants.rst b/doc/modules/constants.rst
index 4cd8b48..a0a2e36 100644
--- a/doc/modules/constants.rst
+++ b/doc/modules/constants.rst
@@ -2,14 +2,14 @@ Constants Definitions
 **************************
 
 
-.. automodule:: pylhc.constants.general
+.. automodule:: pylhc_submitter.constants.general
     :members:
 
 
-.. automodule:: pylhc.constants.external_paths
+.. automodule:: pylhc_submitter.constants.external_paths
     :members:
 
 
-.. automodule:: pylhc.constants.autosix
+.. automodule:: pylhc_submitter.constants.autosix
     :members:
 
diff --git a/doc/modules/htc.rst b/doc/modules/htc.rst
index efbd465..f4965b0 100644
--- a/doc/modules/htc.rst
+++ b/doc/modules/htc.rst
@@ -1,9 +1,9 @@
 HTCondor Tools
 **************************
 
-.. automodule:: pylhc.htc.utils
+.. automodule:: pylhc_submitter.htc.utils
     :members:
 
 
-.. automodule:: pylhc.htc.mask
+.. automodule:: pylhc_submitter.htc.mask
     :members:
diff --git a/doc/modules/sixdesk_tools.rst b/doc/modules/sixdesk_tools.rst
index e4957ed..e9da75d 100644
--- a/doc/modules/sixdesk_tools.rst
+++ b/doc/modules/sixdesk_tools.rst
@@ -1,17 +1,17 @@
 Sixdesk Tools
 **************************
 
-.. automodule:: pylhc.sixdesk_tools.create_workspace
+.. automodule:: pylhc_submitter.sixdesk_tools.create_workspace
     :members:
 
-.. automodule:: pylhc.sixdesk_tools.submit
+.. automodule:: pylhc_submitter.sixdesk_tools.submit
     :members:
 
-.. automodule:: pylhc.sixdesk_tools.post_process_da
+.. automodule:: pylhc_submitter.sixdesk_tools.post_process_da
     :members:
 
-.. automodule:: pylhc.sixdesk_tools.utils
+.. automodule:: pylhc_submitter.sixdesk_tools.utils
     :members:
 
-.. automodule:: pylhc.sixdesk_tools.troubleshooting
+.. automodule:: pylhc_submitter.sixdesk_tools.troubleshooting
     :members:
diff --git a/pylhc_submitter/htc/utils.py b/pylhc_submitter/htc/utils.py
index 8047b8a..80f2bc2 100644
--- a/pylhc_submitter/htc/utils.py
+++ b/pylhc_submitter/htc/utils.py
@@ -15,7 +15,7 @@ job can be specified, standard is 8h.
 import logging
 import subprocess
 from pathlib import Path
-import  sys
+from typing import Union
 
 from pandas import DataFrame
 
@@ -161,6 +161,7 @@ def write_bash(
     output_dir: Path = None,
     executable: str = "madx",
     cmdline_arguments: dict = None,
+    mask: Union[str, Path] = None,
 ) -> DataFrame:
     """Write the bash-files to be called by ``HTCondor``."""
     if len(job_df.index) > HTCONDOR_JOBLIMIT:
@@ -186,7 +187,15 @@ def write_bash(
                 f.write(f"{SHEBANG}\n") 
             if output_dir is not None:
                 f.write(f"mkdir {str(output_dir)}\n")
-            f.write(f"{exec_path}{str(job_dir / job[COLUMN_JOB_FILE])}{cmds}\n")
+            f.write(exec_path)
+
+            if is_mask_file(mask):
+                f.write(str(job_dir / job[COLUMN_JOB_FILE]))
+            else:
+                replace_columns = [column for column in job.index.tolist() if column not in [COLUMN_SHELL_SCRIPT, COLUMN_JOB_DIRECTORY, COLUMN_JOB_FILE]]
+                f.write(mask % dict(zip(replace_columns, job[replace_columns])))
+            f.write(cmds)
+            f.write("\n")
         shell_scripts[idx] = bash_file_name
     job_df[COLUMN_SHELL_SCRIPT] = shell_scripts
     return job_df
@@ -236,6 +245,15 @@ def _maybe_put_in_quotes(key, value):
     return value
 
 
+def is_mask_file(mask):
+    try:
+        return Path(mask).is_file()
+    except OSError:
+        return False
+
+def is_mask_string(mask):
+    return not is_mask_file(mask)
+
 # Script Mode ##################################################################
 
 
diff --git a/pylhc_submitter/job_submitter.py b/pylhc_submitter/job_submitter.py
index 9caf944..1c98102 100644
--- a/pylhc_submitter/job_submitter.py
+++ b/pylhc_submitter/job_submitter.py
@@ -98,10 +98,10 @@ from generic_parser.tools import print_dict_tree
 
 import pylhc_submitter.htc.utils as htcutils
 from pylhc_submitter.htc.mask import (
+    check_percentage_signs_in_mask,
     create_jobs_from_mask,
-    generate_jobdf_index,
     find_named_variables_in_mask,
-    check_percentage_signs_in_mask
+    generate_jobdf_index,
 )
 from pylhc_submitter.htc.utils import (
     COLUMN_JOB_DIRECTORY,
@@ -133,9 +133,11 @@ try:
     HAS_HTCONDOR = True
 except ImportError:
     platform = "macOS" if sys.platform == "darwin" else "windows"
-    LOG.warning(f"htcondor python bindings are linux-only."
-                f" You can still use job_submitter on {platform},"
-                f" but only for local runs.")
+    LOG.warning(
+        f"htcondor python bindings are linux-only."
+        f" You can still use job_submitter on {platform},"
+        f" but only for local runs."
+    )
     HAS_HTCONDOR = False
 
 
@@ -158,8 +160,7 @@ def get_params():
         default="madx",
         type=PathOrStr,
         help=(
-            "Path to executable or job-type "
-            f"(of {str(list(EXECUTEABLEPATH.keys()))}) to use."
+            "Path to executable or job-type " f"(of {str(list(EXECUTEABLEPATH.keys()))}) to use."
         ),
     )
     params.add_parameter(
@@ -175,7 +176,9 @@ def get_params():
         help="Flag to run the jobs on the local machine. Not suggested.",
     )
     params.add_parameter(
-        name="resume_jobs", action="store_true", help="Only do jobs that did not work.",
+        name="resume_jobs",
+        action="store_true",
+        help="Only do jobs that did not work.",
     )
     params.add_parameter(
         name="append_jobs",
@@ -239,7 +242,9 @@ def get_params():
         nargs="+",
     )
     params.add_parameter(
-        name="jobid_mask", help="Mask to name jobs from replace_dict", type=str,
+        name="jobid_mask",
+        help="Mask to name jobs from replace_dict",
+        type=str,
     )
     params.add_parameter(
         name="job_output_dir",
@@ -314,7 +319,7 @@ def main(opt):
 
 def _create_jobs(
     cwd,
-    maskfile,
+    mask_path_or_string,
     jobid_mask,
     replace_dict,
     output_dir,
@@ -322,22 +327,21 @@ def _create_jobs(
     executable,
     script_args,
     script_extension,
-):
-    LOG.debug("Creating Jobs")
+) -> tfs.TfsDataFrame:
+    LOG.debug("Creating Jobs.")
     values_grid = np.array(list(itertools.product(*replace_dict.values())), dtype=object)
 
     if append_jobs:
         jobfile_path = cwd / JOBSUMMARY_FILE
         try:
             job_df = tfs.read(str(jobfile_path.absolute()), index=COLUMN_JOBID)
-        except FileNotFoundError:
+        except FileNotFoundError as filerror:
             raise FileNotFoundError(
                 "Cannot append jobs, as no previous jobfile was found at " f"'{jobfile_path}'"
-            )
+            ) from filerror
         mask = [elem not in job_df[replace_dict.keys()].values for elem in values_grid]
         njobs = mask.count(True)
         values_grid = values_grid[mask]
-
     else:
         njobs = len(values_grid)
         job_df = tfs.TfsDataFrame()
@@ -348,25 +352,28 @@ def _create_jobs(
         raise ValueError(f"Too many jobs! Allowed {HTCONDOR_JOBLIMIT}, given {njobs}.")
 
     LOG.debug(f"Initial number of jobs: {njobs:d}")
-
     data_df = pd.DataFrame(
         index=generate_jobdf_index(job_df, jobid_mask, replace_dict.keys(), values_grid),
         columns=list(replace_dict.keys()),
         data=values_grid,
     )
     job_df = job_df.append(data_df, sort=False)
-
     job_df = _setup_folders(job_df, cwd)
 
-    # creating all madx jobs
-    script_extension = _get_script_extension(script_extension, executable, maskfile)
-    job_df = create_jobs_from_mask(
-        job_df, maskfile, replace_dict.keys(), script_extension
-    )
+    if htcutils.is_mask_file(mask_path_or_string):
+        LOG.debug("Creating all jobs from mask.")
+        script_extension = _get_script_extension(script_extension, executable, mask_path_or_string)
+        job_df = create_jobs_from_mask(
+            job_df, mask_path_or_string, replace_dict.keys(), script_extension
+        )
 
-    # creating all shell scripts
+    LOG.debug("Creating shell scripts for submission.")
     job_df = htcutils.write_bash(
-        job_df, output_dir, executable=executable, cmdline_arguments=script_args
+        job_df,
+        output_dir,
+        executable=executable,
+        cmdline_arguments=script_args,
+        mask=mask_path_or_string,
     )
 
     job_df[COLUMN_JOB_DIRECTORY] = job_df[COLUMN_JOB_DIRECTORY].apply(str)
@@ -374,7 +381,9 @@ def _create_jobs(
     return job_df
 
 
-def _drop_already_run_jobs(job_df, drop_jobs, output_dir, check_files):
+def _drop_already_run_jobs(
+    job_df: tfs.TfsDataFrame, drop_jobs: bool, output_dir: str, check_files: str
+):
     LOG.debug("Dropping already finished jobs, if necessary.")
     finished_jobs = []
     if drop_jobs:
@@ -391,26 +400,35 @@ def _drop_already_run_jobs(job_df, drop_jobs, output_dir, check_files):
     return job_df, finished_jobs
 
 
-def _run_local(job_df, num_processes):
+def _run_local(job_df: tfs.TfsDataFrame, num_processes: int) -> None:
     LOG.info(f"Running {len(job_df.index)} jobs locally in {num_processes:d} processes.")
     pool = multiprocessing.Pool(processes=num_processes)
     res = pool.map(_execute_shell, job_df.iterrows())
     if any(res):
+        LOG.error("At least one job has failed.")
         raise RuntimeError("At least one job has failed. Check output logs!")
 
 
-def _run_htc(job_df, cwd, output_dir, flavour, ssh, dryrun, additional_htc_arguments):
+def _run_htc(
+    job_df: tfs.TfsDataFrame,
+    cwd: str,
+    output_dir: str,
+    flavour: str,
+    ssh: str,
+    dryrun: bool,
+    additional_htc_arguments: DictAsString,
+) -> None:
     LOG.info(f"Submitting {len(job_df.index)} jobs on htcondor, flavour '{flavour}'.")
-    # create submission file
+    LOG.debug("Creating htcondor subfile.")
     subfile = htcutils.make_subfile(
         cwd, job_df, output_dir=output_dir, duration=flavour, **additional_htc_arguments
     )
     if not dryrun:
-        # submit to htcondor
+        LOG.debug("Submitting jobs to htcondor.")
         htcutils.submit_jobfile(subfile, ssh)
 
 
-def _get_script_extension(script_extension, executable, mask):
+def _get_script_extension(script_extension: str, executable: PathOrStr, mask: PathOrStr) -> str:
     if script_extension is not None:
         return script_extension
     return SCRIPT_EXTENSIONS.get(executable, mask.suffix)
@@ -425,7 +443,7 @@ def _check_htcondor_presence() -> None:
         raise EnvironmentError("htcondor bindings are necessary to run this module.")
 
 
-def _setup_folders(job_df, working_directory):
+def _setup_folders(job_df: tfs.TfsDataFrame, working_directory: PathOrStr) -> tfs.TfsDataFrame:
     def _return_job_dir(job_id):
         return working_directory / f"{JOBDIRECTORY_PREFIX}.{job_id}"
 
@@ -442,7 +460,7 @@ def _setup_folders(job_df, working_directory):
     return job_df
 
 
-def _job_was_successful(job_row, output_dir, files):
+def _job_was_successful(job_row, output_dir, files) -> bool:
     output_dir = Path(job_row[COLUMN_JOB_DIRECTORY], output_dir)
     success = output_dir.is_dir() and any(output_dir.iterdir())
     if success and files is not None and len(files):
@@ -451,9 +469,9 @@ def _job_was_successful(job_row, output_dir, files):
     return success
 
 
-def _execute_shell(df_row):
+def _execute_shell(df_row) -> int:
     idx, column = df_row
-    cmd = [] if on_windows() else ['sh']
+    cmd = [] if on_windows() else ["sh"]
 
     with Path(column[COLUMN_JOB_DIRECTORY], "log.tmp").open("w") as logfile:
         process = subprocess.Popen(
@@ -467,18 +485,21 @@ def _execute_shell(df_row):
 
 
 def _check_opts(opt):
-    LOG.debug("Checking options")
+    LOG.debug("Checking options.")
     if opt.resume_jobs and opt.append_jobs:
         raise ValueError("Select either Resume jobs or Append jobs")
 
     # Paths ---
-    opt = keys_to_path(opt, 'mask', 'working_directory', 'executable')
+    opt = keys_to_path(opt, "working_directory", "executable")
 
     if str(opt.executable) in EXECUTEABLEPATH.keys():
         opt.executable = str(opt.executable)
 
-    with open(opt.mask, "r") as inputmask:  # checks that mask and dir are there
-        mask = inputmask.read()
+    if htcutils.is_mask_file(opt.mask):
+        mask = Path(opt.mask).read_text()  # checks that mask and dir are there
+        opt["mask"] = Path(opt["mask"])
+    else:
+        mask = opt.mask
 
     # Replace dict ---
     dict_keys = set(opt.replace_dict.keys())
@@ -506,7 +527,6 @@ def _check_opts(opt):
 
     print_dict_tree(opt, name="Input parameter", print_fun=LOG.debug)
     opt.replace_dict = check_replace_dict(opt.replace_dict)
-
     return opt
 
 
@@ -516,10 +536,10 @@ def _print_stats(new_jobs, finished_jobs):
     LOG.info(f"Jobs total:{len(new_jobs) + len(finished_jobs):d}")
     LOG.info(f"Jobs to run: {len(new_jobs):d}")
     LOG.info(f"Jobs already finished: {len(finished_jobs):d}")
-    LOG.info("---------- JOBS TO RUN NAMES -------------")
+    LOG.info("---------- JOBS TO RUN: NAMES -------------")
     for job_name in new_jobs:
         LOG.info(job_name)
-    LOG.info("--------- JOBS FINISHED NAMES ------------")
+    LOG.info("--------- JOBS FINISHED: NAMES ------------")
     for job_name in finished_jobs:
         LOG.info(job_name)
 


Test patch:
diff --git a/tests/unit/test_job_submitter.py b/tests/unit/test_job_submitter.py
index 3483671..2b7c43b 100644
--- a/tests/unit/test_job_submitter.py
+++ b/tests/unit/test_job_submitter.py
@@ -17,17 +17,27 @@ run_if_not_linux = pytest.mark.skipif(
     on_linux(), reason="htcondor python bindings are present"
 )
 
+@pytest.mark.parametrize("maskfile", [True, False])
+def test_job_creation_and_localrun(tmp_path, maskfile):
+    args, setup = _create_setup(tmp_path, mask_file=maskfile)
+    setup.update(run_local=True)
+    job_submit(**setup)
+    _test_output(args)
 
-def test_job_creation_and_localrun(tmp_path):
-    args, setup = _create_setup(tmp_path)
+
+@run_only_on_linux
+def test_job_creation_and_localrun_with_multiline_maskstring(tmp_path):
+    mask = "123\"\" \nsleep 0.1 \n/bin/bash -c  \"echo \"%(PARAM1)s.%(PARAM2)s"
+    args, setup = _create_setup(tmp_path, mask_content=mask, mask_file=False)
     setup.update(run_local=True)
     job_submit(**setup)
     _test_output(args)
 
 
 @run_only_on_linux
-def test_job_creation_and_dryrun(tmp_path):
-    args, setup = _create_setup(tmp_path)
+@pytest.mark.parametrize("maskfile", [True, False])
+def test_job_creation_and_dryrun(tmp_path, maskfile):
+    args, setup = _create_setup(tmp_path, mask_file=maskfile)
     setup.update(dryrun=True)
     job_submit(**setup)
     _test_subfile_content(setup)
@@ -35,18 +45,20 @@ def test_job_creation_and_dryrun(tmp_path):
 
 
 @run_only_on_linux
-def test_find_errorneous_percentage_signs(tmp_path):
+@pytest.mark.parametrize("maskfile", [True, False])
+def test_find_errorneous_percentage_signs(tmp_path, maskfile):
     mask = "%(PARAM1)s.%(PARAM2)d\nsome stuff # should be 5%\nsome % more % stuff."
-    args, setup = _create_setup(tmp_path, mask_content=mask)
+    args, setup = _create_setup(tmp_path, mask_content=mask, mask_file=maskfile)
     with pytest.raises(KeyError) as e:
         job_submit(**setup)
     assert "problematic '%'" in e.value.args[0]
 
 
 @run_only_on_linux
-def test_missing_keys(tmp_path):
+@pytest.mark.parametrize("maskfile", [True, False])
+def test_missing_keys(tmp_path, maskfile):
     mask = "%(PARAM1)s.%(PARAM2)s.%(PARAM3)s"
-    args, setup = _create_setup(tmp_path, mask_content=mask)
+    args, setup = _create_setup(tmp_path, mask_content=mask, mask_file=maskfile)
     with pytest.raises(KeyError) as e:
         job_submit(**setup)
     assert "PARAM3" in e.value.args[0]
@@ -79,7 +91,7 @@ def test_htc_submit():
 # Helper -----------------------------------------------------------------------
 
 
-def _create_setup(cwd_path: Path, mask_content: str = None):
+def _create_setup(cwd_path: Path, mask_content: str = None, mask_file: bool = True):
     """ Create a quick setup for Parameters PARAM1 and PARAM2. """
     out_name = "out.txt"
     out_dir = "Outputdir"
@@ -94,23 +106,20 @@ def _create_setup(cwd_path: Path, mask_content: str = None):
         out_file=Path(out_dir, out_name),
         p1_list=["a", "b"],
         p2_list=[1, 2, 3],
+        mask_file=mask_file
     )
 
-    mask_path = cwd_path / args.mask_name
-    if mask_content is None:
-        mask_content = args.id
-
-    with mask_path.open("w") as f:
-        if on_windows():
-            f.write(f'echo {mask_content}> "{args.out_file}"\n')
-        else:
-            f.write(f'echo "{mask_content}" > "{args.out_file}"\n')
+    mask_string = _make_executable_string(args, mask_content)
+    if args.mask_file:
+        mask_path = args.cwd / args.mask_name
+        with mask_path.open("w") as f:
+            f.write(mask_string)
 
     setup = dict(
         executable=None if on_windows() else "/bin/bash",
         script_extension=args.ext,
         job_output_dir=out_dir,
-        mask=str(mask_path),
+        mask=str(mask_path) if args.mask_file else mask_string,
         replace_dict=dict(PARAM1=args.p1_list, PARAM2=args.p2_list,),
         jobid_mask=args.id,
         jobflavour="workday",
@@ -124,6 +133,18 @@ def _create_setup(cwd_path: Path, mask_content: str = None):
     )
     return args, setup
 
+def _make_executable_string(args, mask_content):
+    if mask_content is None:
+        mask_content = args.id
+
+    if on_windows():
+        mask_string=f'echo {mask_content}> "{args.out_file}"'
+    else:
+        mask_string=f'echo "{mask_content}" > "{args.out_file}"'
+        if not args.mask_file:
+            mask_string = ' '.join(['-c "', mask_string, '"'])
+    return f'{mask_string}\n'
+
 
 def _test_subfile_content(setup):
     subfile = Path(setup['working_directory']) / SUBFILE
@@ -147,7 +168,8 @@ def _test_output(args, post_run=True):
 
             assert job_dir_path.exists()
             assert job_dir_path.is_dir()
-            assert (job_dir_path / args.mask_name).with_suffix(args.ext).exists()
+            if args.mask_file:
+                assert (job_dir_path / args.mask_name).with_suffix(args.ext).exists()
             # assert out_dir_path.exists()  # does not seem to be pre-created anymore (jdilly 2021-05-04)
             if post_run:
                 assert out_dir_path.is_dir()