[ASP-3985] Add extra params create-job-script (#387)

* Create question helper

* Add cluster_name, exec_dir and download to render

* Update logic for default execution_dir

* Add unit tests

* Lint code

* Requested changes in code review

* Update logic for download

* Add entry to CHANGELOG

jobbergate-cli/CHANGELOG.md

@@ -3,16 +3,17 @@
 This file keeps track of all notable changes to jobbergate-cli
 
 ## Unreleased
+- Added --cluster-name, --execution-directory and --download parameters to create-job-script command on submit mode
 
 
 ## 4.1.0a2 -- 2023-10-10
 - Added a `create` for Job Scripts to to create without Template. (former `create` renamed to `render`)
 - Fixed help information for id on `get-job-script`, `download-job-script`, and `get-job-submission` commands
 - Added short arguments for backward compatiblity
-- Ignore username and password arguments if provided, aiming to keep backward compatibility
-- Add support to select applications by identifier in update and delete commands
-- Add show-files command to compat mode
-- Add support to select all and deselect all options in checkboxes using Ctrl+A and Ctrl+R as shortcuts
+- Ignored username and password arguments if provided, aiming to keep backward compatibility
+- Added support to select applications by identifier in update and delete commands
+- Added show-files command to compat mode
+- Added support to select all and deselect all options in checkboxes using Ctrl+A and Ctrl+R as shortcuts
 
 
 ## 4.1.0a1 -- 2023-10-02

jobbergate-cli/jobbergate_cli/subapps/applications/questions.py

@@ -201,7 +201,7 @@ def __init__(self, variablename: str, message: str, choices: list, **kwargs):
             variablename,
             message + " (press CTRL+A to SELECT ALL or CTRL+R to DESELECT ALL)",
             inquirer_type=inquirer.Checkbox,
-            **kwargs
+            **kwargs,
         )
         self.inquirer_kwargs.update(choices=choices)
 

jobbergate-cli/jobbergate_cli/subapps/job_scripts/app.py

@@ -8,6 +8,7 @@
 
 import typer
 
+from jobbergate_cli.config import settings
 from jobbergate_cli.constants import SortOrder
 from jobbergate_cli.exceptions import Abort, handle_abort
 from jobbergate_cli.render import StyleMapper, render_list_results, render_single_result, terminal_message
@@ -16,6 +17,7 @@
 from jobbergate_cli.subapps.job_scripts.tools import (
     download_job_script_files,
     fetch_job_script_data,
+    question_helper,
     render_job_script,
     save_job_script_files,
     upload_job_script_files,
@@ -210,6 +212,25 @@ def render(
             """
         ),
     ),
+    cluster_name: Optional[str] = typer.Option(
+        None,
+        help="The name of the cluster where the job should be submitted (i.g. 'nash-staging')",
+    ),
+    execution_directory: Optional[pathlib.Path] = typer.Option(
+        None,
+        help=dedent(
+            """
+            The path on the cluster where the job script should be executed.
+            If provided as a relative path, it will be converted as an absolute path from your current
+            working directory. If you use "~" to denote your home directory, the path will be expanded to an
+            absolute path for your home directory on *this* machine.
+            """
+        ).strip(),
+    ),
+    download: Optional[bool] = typer.Option(
+        None,
+        help="Download the job script files to the current working directory",
+    ),
     fast: bool = typer.Option(
         False,
         "--fast",
@@ -244,21 +265,70 @@ def render(
         title="Created Job Script",
     )
 
-    # `submit` will be `None` --submit/--no-submit flag was not set
-    if submit is None:
-        # If not running in "fast" mode, ask the user what to do.
-        if not fast:
-            submit = typer.confirm("Would you like to submit this job immediately?")
+    # `submit` will be `None` when --submit/--no-submit flag was not set
+    if submit is None and (cluster_name or execution_directory):
+        # If the user specified --cluster-name or --execution-directory, they must also specify --submit
+        raise Abort(
+            "You must specify --cluster-name and --execution-directory only on submit mode.",
+            subject="Incorrect parameters",
+            support=True,
+        )
+
+    """
+    Invoke `question_helper` to handle the logic for parameters that are not specified by the user.
+
+    If the user specified --fast, the `question_helper` will not invoke the function and will
+    instead return the default answer. If the user did not specify --fast, the `question_helper`
+    will invoke the function and return the user's answer.
+    """
+
+    submit = question_helper(
+        question_func=typer.confirm,
+        text="Would you like to submit this job immediately?",
+        default=True,
+        fast=fast,
+        actual_value=submit,
+    )
+
+    download = question_helper(
+        question_func=typer.confirm,
+        text="Would you like to download the job script files?",
+        default=True,
+        fast=fast,
+        actual_value=download,
+    )
 
-        # Otherwise, assume that the job script should be submitted immediately
-        else:
-            submit = True
+    if download:
+        download_job_script_files(job_script_result.id, jg_ctx)
 
     if not submit:
         return
 
+    cluster_name = question_helper(
+        question_func=typer.prompt,
+        text="What cluster should this job be submitted to?",
+        default=settings.DEFAULT_CLUSTER_NAME,
+        fast=fast,
+        actual_value=cluster_name,
+    )
+    execution_directory = question_helper(
+        question_func=typer.prompt,
+        text="Where should this job be executed?",
+        default=pathlib.Path.cwd(),
+        fast=fast,
+        actual_value=execution_directory,
+    )
+
     try:
-        job_submission_result = create_job_submission(jg_ctx, job_script_result.id, job_script_result.name)
+        job_submission_result = create_job_submission(
+            jg_ctx,
+            job_script_result.id,
+            job_script_result.name,
+            job_script_result.description,
+            cluster_name,
+            execution_directory,
+            param_file,
+        )
     except Exception as err:
         raise Abort(
             "Failed to immediately submit the job after job script creation.",

jobbergate-cli/jobbergate_cli/subapps/job_scripts/tools.py

@@ -6,7 +6,7 @@
 import pathlib
 import re
 import tempfile
-from typing import Any, Dict, List, Optional, cast
+from typing import Any, Callable, Dict, List, Optional, cast
 
 from loguru import logger
 
@@ -386,3 +386,26 @@ def download_job_script_files(id: int, jg_ctx: JobbergateContext) -> List[pathli
         destination_path=pathlib.Path.cwd(),
     )
     return downloaded_files
+
+
+def question_helper(question_func: Callable, text: str, default: Any, fast: bool, actual_value: Optional[Any]):
+    """
+    Helper function for asking questions to the user.
+
+    :param Callable question_func: The function to use to ask the question
+    :param str text:               The text of the question to ask
+    :param Any default:            The default value to use if the user does not provide one
+    :param bool fast:              Whether to use default answers (when available) instead of asking the user
+    :param Any actual_value:       The actual value provided by the user, if any
+
+    :returns: `actual_value` or `default` or the value provided by the user
+
+    The `actual_value` has the most priority and will be returned if it is not None.
+    After evaluating the `actual_value`, the fast mode will determine if the default value will be used.
+    Otherwise, the question will be prompted to the user.
+    """
+    if actual_value is not None:
+        return actual_value
+    if fast:
+        return default
+    return question_func(text, default=default)

jobbergate-cli/jobbergate_cli/subapps/job_submissions/tools.py

@@ -24,16 +24,19 @@ def create_job_submission(
     """
     Create a Job Submission from the given Job Script.
 
-    :param: jg_ctx:              The JobbergateContext. Used to retrieve the client for requests
-                                 and the email of the submitting user
-    :param: job_script_id:       The ``id`` of the Job Script to submit to Slurm
-    :param: name:                The name to attach to the Job Submission
-    :param: description:         An optional description that may be added to the Job Submission
-    :param: cluster_name:        An optional cluster_name for the cluster where the job should be executed,
-                                 If left off, it will default to the DEFAULT_CLUSTER_NAME from the settings.
-                                 If no default is set, an exception will be raised.
-    :param: execution_directory: An optional directory where the job should be executed. If provided as a relative path,
-                                 it will be constructed as an absolute path relative to the current working directory.
+    :param: jg_ctx:                    The JobbergateContext. Used to retrieve the client for requests
+                                       and the email of the submitting user
+    :param: job_script_id:             The ``id`` of the Job Script to submit to Slurm
+    :param: name:                      The name to attach to the Job Submission
+    :param: description:               An optional description that may be added to the Job Submission
+    :param: cluster_name:              An optional cluster_name for the cluster where the job should be executed,
+                                       If left off, it will default to the DEFAULT_CLUSTER_NAME from the settings.
+                                       If no default is set, an exception will be raised.
+    :param: execution_directory:       An optional directory where the job should be executed. If provided as a
+                                       relative path, it will be constructed as an absolute path relative to
+                                       the current working directory.
+    :param: execution_parameters_file: An optional file containing the execution parameters for the job.
+
     :returns: The Job Submission data returned by the API after creating the new Job Submission
     """
 
@@ -60,11 +63,12 @@ def create_job_submission(
         cluster_name=cluster_name,
     )
 
-    if execution_directory is not None:
+    if execution_directory is None:
+        execution_directory = Path.cwd()
+    else:
         if not execution_directory.is_absolute():
-            execution_directory = Path.cwd() / execution_directory
-        execution_directory.resolve()
-        job_submission_data.execution_directory = execution_directory
+            execution_directory = (Path.cwd() / execution_directory).resolve()
+    job_submission_data.execution_directory = execution_directory
 
     if execution_parameters_file is not None:
         job_submission_data.execution_parameters = validate_parameter_file(execution_parameters_file)

jobbergate-cli/tests/subapps/job_scripts/test_app.py

@@ -6,6 +6,7 @@
 import httpx
 import pytest
 
+from jobbergate_cli.config import settings
 from jobbergate_cli.schemas import (
     ApplicationResponse,
     JobScriptFiles,
@@ -291,7 +292,8 @@ def test_render__non_fast_mode_and_job_submission(
                 """
             )
         ),
-        input="y\n",  # To confirm that the job should be submitted right away
+        # To confirm that the job should be submitted to the default cluster, in the current dir and not downloaded
+        input=f"y\nn\n{settings.DEFAULT_CLUSTER_NAME}\n.\n",
     )
     assert result.exit_code == 0, f"render failed: {result.stdout}"
     assert mocked_fetch_application_data.called_once_with(
@@ -411,6 +413,7 @@ def test_render__with_fast_mode_and_no_job_submission(
                        --param-file={param_file_path}
                        --fast
                        --no-submit
+                       --no-download
                        {sbatch_params}
                 """
             )
@@ -453,6 +456,77 @@ def test_render__with_fast_mode_and_no_job_submission(
     )
 
 
+def test_render__submit_is_none_and_cluster_name_is_defined(
+    respx_mock,
+    make_test_app,
+    dummy_context,
+    dummy_module_source,
+    dummy_application_data,
+    dummy_job_script_data,
+    dummy_domain,
+    cli_runner,
+    tmp_path,
+    attach_persona,
+    mocker,
+):
+    application_response = ApplicationResponse(**dummy_application_data[0])
+
+    job_script_data = dummy_job_script_data[0]
+
+    render_route = respx_mock.post(
+        f"{dummy_domain}/jobbergate/job-scripts/render-from-template/{application_response.id}"
+    )
+    render_route.mock(
+        return_value=httpx.Response(
+            httpx.codes.CREATED,
+            json=job_script_data,
+        ),
+    )
+
+    sbatch_params = " ".join(f"--sbatch-params={i}" for i in (1, 2, 3))
+
+    param_file_path = tmp_path / "param_file.json"
+    param_file_path.write_text(
+        json.dumps(
+            dict(
+                foo="oof",
+                bar="rab",
+                baz="zab",
+            )
+        )
+    )
+
+    attach_persona("dummy@dummy.com")
+
+    test_app = make_test_app("render", render)
+    mocked_render = mocker.patch("jobbergate_cli.subapps.job_scripts.app.render_single_result")
+    mocked_abort = mocker.patch("jobbergate_cli.subapps.job_scripts.app.Abort")
+
+    cli_runner.invoke(
+        test_app,
+        shlex.split(
+            unwrap(
+                f"""
+                render --name=dummy-name
+                       --application-id={application_response.id}
+                       --param-file={param_file_path}
+                       --cluster-name=dummy-cluster
+                       {sbatch_params}
+                """
+            )
+        ),
+    )
+    assert mocked_render.called_once_with(
+        dummy_context,
+        JobScriptResponse(**job_script_data),
+        title="Created Job Script",
+        hidden_fields=HIDDEN_FIELDS,
+    )
+    assert mocked_abort.called_once_with(
+        "You must specify --cluster-name and --execution-directory only on submit mode.",
+    )
+
+
 @pytest.mark.parametrize("id_flag,separator", [("--id", "="), ("-i", " ")])
 def test_update__makes_request_and_renders_results(
     respx_mock,