diff --git a/CHANGES.md b/CHANGES.md index 7444b2e..748b9bd 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -7,6 +7,7 @@ * Improve type annotations and checks (#68) * Include Dockerfile in built images (#55) * Look for environment.yml automatically (#41) +* Allow 'xcengine image run' to pass arguments to the container (#57) ## Changes in 0.1.1 diff --git a/docs/xcetool.md b/docs/xcetool.md index a3cd4ba..6506036 100644 --- a/docs/xcetool.md +++ b/docs/xcetool.md @@ -39,7 +39,7 @@ Options: ### `xcetool image run` -Usage: `xcetool image run [OPTIONS] IMAGE` +Usage: `xcetool image run [OPTIONS] IMAGE [CONTAINER_ARGUMENT]...` Options: @@ -59,9 +59,11 @@ Options: running. - `--help`: Show a help message for this subcommand and exit. -This subcommand runs an xcengine container image. An image can also be run -using the `docker run` command, but `xcetool image run` provides some -additional convenience (e.g. easy configuration of a server HTTP port). +This subcommand runs an xcengine container image. Any arguments provided +after IMAGE will be passed on to the command executed inside the container. +An image can also be run using the `docker run` command, but +`xcetool image run` provides some additional convenience (e.g. easy +configuration of a server HTTP port). If you use the `--server` option with `xcetool image run`, the image will be run in xcube server mode: after the code from the input notebook is used to diff --git a/environment.yml b/environment.yml index d239c7a..13a1707 100644 --- a/environment.yml +++ b/environment.yml @@ -18,6 +18,7 @@ dependencies: - cwltool - pytest - pytest-cov + - pytz # Note: xcube is not required for the conversion itself, but is required # to run generated scripts outside containers ("create" mode). xcube is diff --git a/pyproject.toml b/pyproject.toml index 8937cd2..2dbcc8d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -56,7 +56,8 @@ xcetool = "xcengine.cli:cli" dev = [ "cwltool", "pytest", - "pytest-cov" + "pytest-cov", + "pytz" ] doc = [ "mkdocs", diff --git a/test/test_cli.py b/test/test_cli.py index a48496d..a265963 100644 --- a/test/test_cli.py +++ b/test/test_cli.py @@ -102,6 +102,7 @@ def test_image_run(runner_mock): host_port=None, from_saved=False, keep=False, + script_args=[], ) @@ -120,6 +121,7 @@ def test_image_run_print_urls(runner_mock): host_port=port, from_saved=False, keep=False, + script_args=[], ) assert re.search( f"server.*http://localhost:{port}", result.stdout, re.IGNORECASE @@ -127,3 +129,22 @@ def test_image_run_print_urls(runner_mock): assert re.search( f"viewer.*http://localhost:{port}/viewer", result.stdout, re.IGNORECASE ) + + +@patch("xcengine.cli.ContainerRunner") +def test_image_run_script_args(runner_mock): + cli_runner = CliRunner() + instance_mock = runner_mock.return_value = MagicMock() + port = 32168 + result = cli_runner.invoke( + cli, ["image", "run", "--server", "--port", str(port), "foo", "--bar"] + ) + runner_mock.assert_called_once_with(image="foo", output_dir=None) + assert result.exit_code == 0 + instance_mock.run.assert_called_once_with( + run_batch=False, + host_port=port, + from_saved=False, + keep=False, + script_args=["--bar"], + ) diff --git a/test/test_core.py b/test/test_core.py index 486b26e..a58ae65 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -29,7 +29,9 @@ @patch("xcengine.core.ScriptCreator.__init__") @pytest.mark.parametrize("tag", [None, "bar"]) -@pytest.mark.parametrize("env_file_name", ["environment.yml", "foo.yaml", None]) +@pytest.mark.parametrize( + "env_file_name", ["environment.yml", "foo.yaml", None] +) @pytest.mark.parametrize("use_env_file_param", [False, True]) def test_image_builder_init( init_mock, @@ -56,7 +58,11 @@ def test_image_builder_init( ) assert ib.notebook == nb_path assert ib.build_dir == build_path - expected_env = environment_path if (use_env_file_param or env_file_name == "environment.yml") else None + expected_env = ( + environment_path + if (use_env_file_param or env_file_name == "environment.yml") + else None + ) assert ib.environment == expected_env if tag is None: assert abs( @@ -123,6 +129,28 @@ def test_runner_run_keep(keep: bool): container.remove.assert_called_once_with(force=True) +def test_runner_extra_args(): + runner = xcengine.core.ContainerRunner( + image := Mock(docker.models.images.Image), + None, + client := Mock(DockerClient), + ) + image.tags = [] + client.containers.run.return_value = (container := MagicMock(Container)) + container.status = "exited" + script_args = ["--foo", "--bar", "42", "--baz", "somestring"] + runner.run( + run_batch=False, + host_port=None, + from_saved=False, + keep=False, + script_args=script_args, + ) + run_args = client.containers.run.call_args + command = run_args[1]["command"] + assert command == ["python", "execute.py"] + script_args + + def test_runner_sigint(): runner = xcengine.core.ContainerRunner( image := Mock(docker.models.images.Image), diff --git a/xcengine/cli.py b/xcengine/cli.py index 125156f..13540d9 100644 --- a/xcengine/cli.py +++ b/xcengine/cli.py @@ -103,7 +103,7 @@ def image_cli(): @image_cli.command( help="Build a compute engine as a Docker image, optionally generating an " - "Application Package" + "Application Package" ) @click.option( "-b", @@ -119,7 +119,7 @@ def image_cli(): help="Conda environment file to use in Docker image. " "If no environment file is specified here or in the notebook, and if " "there is no file named environment.yml in the notebook's directory, " - "xcetool will try to reproduce the current environment." + "xcetool will try to reproduce the current environment.", ) @click.option( "-t", @@ -147,10 +147,12 @@ def build( ) -> None: if environment is None: LOGGER.info("No environment file specified on command line.") + class InitArgs(TypedDict): notebook: pathlib.Path environment: pathlib.Path tag: str + init_args = InitArgs(notebook=notebook, environment=environment, tag=tag) if build_dir: image_builder = ImageBuilder(build_dir=build_dir, **init_args) @@ -163,9 +165,11 @@ class InitArgs(TypedDict): ) image = image_builder.build() if eoap: + class IndentDumper(yaml.Dumper): def increase_indent(self, flow=False, indentless=False): return super(IndentDumper, self).increase_indent(flow, False) + eoap.write_text( yaml.dump( image_builder.create_cwl(), @@ -176,7 +180,14 @@ def increase_indent(self, flow=False, indentless=False): print(f"Built image with tags {image.tags}") -@image_cli.command(help="Run a compute engine image as a Docker container.") +@image_cli.command( + help="Run a compute engine image as a Docker container. " + "Any arguments provided after IMAGE will be passed on to the command " + "executed inside the container.", + context_settings=dict( + ignore_unknown_options=True, + ), +) @click.option( "-b", "--batch", @@ -213,6 +224,12 @@ def increase_indent(self, flow=False, indentless=False): help="Keep container after it has finished running.", ) @click.argument("image", type=str) +@click.argument( + "script_args", + nargs=-1, + type=click.UNPROCESSED, + metavar="[CONTAINER_ARGUMENT]...", +) @click.pass_context def run( ctx: click.Context, @@ -223,6 +240,7 @@ def run( keep: bool, image: str, output: pathlib.Path, + script_args, ) -> None: runner = ContainerRunner(image=image, output_dir=output) port_specified_explicitly = ( @@ -243,4 +261,5 @@ def run( host_port=actual_port, from_saved=from_saved, keep=keep, + script_args=list(script_args), ) diff --git a/xcengine/core.py b/xcengine/core.py index 73a1d85..15684a2 100755 --- a/xcengine/core.py +++ b/xcengine/core.py @@ -33,6 +33,7 @@ LOGGER = logging.getLogger(__name__) logging.basicConfig(level=logging.INFO) + class ScriptCreator: """Turn a Jupyter notebook into a set of scripts""" @@ -221,7 +222,7 @@ def __init__( self.environment = notebook.parent / nb_env else: LOGGER.info(f"No environment specified in notebook.") - LOGGER.info(f"Looking for a file named \"environment.yml\".") + LOGGER.info(f'Looking for a file named "environment.yml".') notebook_sibling = notebook.parent / "environment.yml" if notebook_sibling.is_file(): self.environment = notebook_sibling @@ -375,9 +376,11 @@ def run( host_port: int | None, from_saved: bool, keep: bool, + script_args: list[str] | None = None, ): LOGGER.info(f"Running container from image {self.image.short_id}") LOGGER.info(f"Image tags: {' '.join(self.image.tags)}") + assert isinstance(script_args, list) or script_args is None command = ( ["python", "execute.py"] + (["--batch"] if run_batch else []) @@ -391,6 +394,9 @@ def run( else [] ) + (["--from-saved"] if from_saved else []) + + script_args + if script_args is not None + else [] ) run_args: dict[str, Any] = dict( image=self.image, command=command, remove=False, detach=True @@ -400,10 +406,14 @@ def run( container: Container = self.client.containers.run(**run_args) LOGGER.info(f"Waiting for container {container.short_id} to complete.") default_sigint_handler = signal.getsignal(signal.SIGINT) + def signal_hander(signum, frame): signal.signal(signal.SIGINT, default_sigint_handler) - LOGGER.info(f"Caught SIGINT. Stopping container {container.short_id}") + LOGGER.info( + f"Caught SIGINT. Stopping container {container.short_id}" + ) container.stop() + signal.signal(signal.SIGINT, signal_hander) while container.status in {"created", "running"}: LOGGER.debug(