Merge pull request #4 from xcube-dev/pont-3-port-option

Add port option, and remove run functionality from build subcommand

Author: pont-us

LICENSE

@@ -1,6 +1,6 @@
 MIT License
 
-Copyright (c) 2024 Brockmann Consult GmbH
+Copyright (c) 2024-2025 Brockmann Consult GmbH
 
 Permission is hereby granted, free of charge, to any person obtaining a copy
 of this software and associated documentation files (the "Software"), to deal

examples/environment.yml

@@ -3,5 +3,5 @@ channels:
   - conda-forge
 dependencies:
   - python >=3.10
-  - xcube >=1.7.1
+  - xcube >=1.8.3
   - matplotlib-base

test/test_cli.py

@@ -1,5 +1,4 @@
-from unittest.mock import patch
-
+from unittest.mock import patch, ANY
 import pytest
 from click.testing import CliRunner
 
@@ -10,35 +9,64 @@
 @patch("xcengine.core.ScriptCreator.convert_notebook_to_script")
 @patch("subprocess.run")
 @pytest.mark.parametrize("verbose_arg", [[], ["--verbose"]])
-def test_make_script(run_mock, convert_mock, init_mock, tmp_path, verbose_arg):
+@pytest.mark.parametrize("batch_arg", [[], ["--batch"]])
+@pytest.mark.parametrize("server_arg", [[], ["--server"]])
+@pytest.mark.parametrize("from_saved_arg", [[], ["--from-saved"]])
+def test_make_script(
+    run_mock,
+    convert_mock,
+    init_mock,
+    tmp_path,
+    verbose_arg,
+    batch_arg,
+    server_arg,
+    from_saved_arg,
+):
+    from xcengine.cli import logging
+
+    logging.getLogger().setLevel(logging.WARN)
     nb_path = tmp_path / "foo.ipynb"
     nb_path.touch()
     output_dir = tmp_path / "bar"
     init_mock.return_value = None
     runner = CliRunner()
     result = runner.invoke(
         cli,
-        verbose_arg +
-        [
-            "make-script",
-            "--batch",
-            "--server",
-            "--from-saved",
+        verbose_arg
+        + ["make-script"]
+        + batch_arg
+        + server_arg
+        + from_saved_arg
+        + [
             str(nb_path),
             str(output_dir),
         ],
     )
-    convert_mock.assert_called()
-    init_mock.assert_called()
-    run_mock.assert_called_with(
-        [
-            "python3",
-            output_dir / "execute.py",
-            "--batch",
-            "--server",
-            "--from-saved",
-        ]
+    convert_mock.assert_called_once_with(
+        output_dir=output_dir, clear_output=False
+    )
+    init_mock.assert_called_once_with(nb_path)
+    if batch_arg or server_arg:
+        run_mock.assert_called_once_with(
+            ["python3", output_dir / "execute.py"]
+            + batch_arg
+            + server_arg
+            + from_saved_arg
+        )
+    assert logging.getLogger().getEffectiveLevel() == (
+        logging.DEBUG if "--verbose" in verbose_arg else logging.WARNING
     )
-    from xcengine.cli import logging
-    assert logging.getLogger().getEffectiveLevel() == (logging.DEBUG if "--verbose" in verbose_arg else logging.WARNING)
     assert result.exit_code == 0
+
+
+@patch("xcengine.core.ImageBuilder.__init__")
+@patch("xcengine.core.ImageBuilder.build")
+def test_image_build(build_mock, init_mock, tmp_path):
+    nb_path = tmp_path / "foo.ipynb"
+    nb_path.touch()
+    runner = CliRunner()
+    tag = "foo"
+    result = runner.invoke(cli, ["image", "build", "--tag", tag, str(nb_path)])
+    init_mock.assert_called_once_with(
+        notebook=nb_path, environment=None, tag=tag, build_dir=ANY
+    )

xcengine/cli.py

@@ -1,6 +1,6 @@
 #!/usr/bin/env python3
 
-# Copyright (c) 2024 by Brockmann Consult GmbH
+# Copyright (c) 2024-2025 by Brockmann Consult GmbH
 # Permissions are hereby granted under the terms of the MIT License:
 # https://opensource.org/licenses/MIT.
 
@@ -12,6 +12,7 @@
 
 import click
 import yaml
+from click.core import ParameterSource
 
 from .core import ScriptCreator, ImageBuilder, ContainerRunner
 
@@ -30,13 +31,6 @@ def cli(verbose):
     "-b", "--batch", is_flag=True, help="Run as batch script after creating"
 )
 
-server_option = click.option(
-    "-s",
-    "--server",
-    is_flag=True,
-    help="Run as xcube server script after creating",
-)
-
 output_option = click.option(
     "-o",
     "--output",
@@ -66,6 +60,13 @@ def cli(verbose):
     ),
 )
 
+server_option = click.option(
+    "-s",
+    "--server",
+    is_flag=True,
+    help="Run the script as an xcube server after creating it.",
+)
+
 
 @cli.command(help="Create a compute engine script on the host system")
 @batch_option
@@ -113,25 +114,20 @@ def image_cli():
 @image_cli.command(
     help="Build, and optionally run, a compute engine as a Docker image"
 )
-@batch_option
-@server_option
-@from_saved_option
 @click.option(
     "-b",
     "--build-dir",
     type=click.Path(path_type=pathlib.Path, dir_okay=True, file_okay=False),
     help="Build directory to use for preparing the Docker image. If not "
     "specified, an automatically created temporary directory will be used.",
 )
-@keep_option
 @click.option(
     "-e",
     "--environment",
     type=click.Path(path_type=pathlib.Path, dir_okay=False, file_okay=True),
     help="Conda environment file to use in Docker image. "
     "If not specified, try to reproduce the current environment.",
 )
-@output_option
 @click.option(
     "-t",
     "--tag",
@@ -150,55 +146,65 @@ def image_cli():
 )
 @notebook_argument
 def build(
-    batch: bool,
-    server: bool,
-    from_saved: bool,
-    keep: bool,
     build_dir: pathlib.Path,
     notebook: pathlib.Path,
-    output: pathlib.Path,
     environment: pathlib.Path,
     tag: str,
     eoap: pathlib.Path,
 ) -> None:
-    init_args = dict(
-        notebook=notebook, output_dir=output, environment=environment, tag=tag
-    )
-    build_args = dict(
-        run_batch=batch, run_server=server, from_saved=from_saved, keep=keep
-    )
+    init_args = dict(notebook=notebook, environment=environment, tag=tag)
     if build_dir:
         image_builder = ImageBuilder(build_dir=build_dir, **init_args)
         os.makedirs(build_dir, exist_ok=True)
-        image_builder.build(**build_args)
+        image = image_builder.build()
     else:
         with tempfile.TemporaryDirectory() as temp_dir:
             image_builder = ImageBuilder(
                 build_dir=pathlib.Path(temp_dir), **init_args
             )
-            image_builder.build(**build_args)
+            image = image_builder.build()
     if eoap:
         eoap.write_text(yaml.dump(image_builder.create_cwl()))
+    print(f"Built image with tags {image.tags}")
 
 
 @image_cli.command(help="Run a compute engine image as a Docker container")
 @batch_option
 @server_option
+@click.option(
+    "-p",
+    "--port",
+    is_flag=False,
+    type=int,
+    default=8080,
+    help="Host port for xcube server (default: 8080). Implies --server.",
+)
 @from_saved_option
 @output_option
 @keep_option
 @click.argument("image", type=str)
+@click.pass_context
 def run(
+    ctx: click.Context,
     batch: bool,
-    server: bool,
+    server: False,
+    port: int,
     from_saved: bool,
     keep: bool,
     image: str,
     output: pathlib.Path,
 ) -> None:
     runner = ContainerRunner(image=image, output_dir=output)
+    port_specified_explicitly = (
+        ctx.get_parameter_source("port")
+        is not click.core.ParameterSource.DEFAULT
+    )
+    actual_port = port if server or port_specified_explicitly else None
     runner.run(
-        run_batch=batch, run_server=server, from_saved=from_saved, keep=keep
+        run_batch=batch,
+        host_port=actual_port,
+        from_saved=from_saved,
+        keep=keep,
     )
 
 

xcengine/core.py

@@ -1,4 +1,4 @@
-# Copyright (c) 2024 by Brockmann Consult GmbH
+# Copyright (c) 2024-2025 by Brockmann Consult GmbH
 # Permissions are hereby granted under the terms of the MIT License:
 # https://opensource.org/licenses/MIT.
 
@@ -153,13 +153,11 @@ class ImageBuilder:
     def __init__(
         self,
         notebook: pathlib.Path,
-        output_dir: pathlib.Path,
         environment: pathlib.Path,
         build_dir: pathlib.Path,
         tag: str | None,
     ):
         self.notebook = notebook
-        self.output_dir = output_dir
         self.environment = environment
         self.build_dir = build_dir
         if tag is None:
@@ -171,13 +169,7 @@ def __init__(
             self.tag = tag
         self.script_creator = ScriptCreator(self.notebook)
 
-    def build(
-        self,
-        run_batch: bool,
-        run_server: bool,
-        from_saved: bool,
-        keep: bool,
-    ) -> None:
+    def build(self) -> Image:
         self.script_creator.convert_notebook_to_script(self.build_dir)
         if self.environment:
             with open(self.environment, "r") as fh:
@@ -192,10 +184,7 @@ def build(
         self.add_packages_to_environment(env_def, ["xcube", "pystac"])
         with open(self.build_dir / "environment.yml", "w") as fh:
             fh.write(yaml.safe_dump(env_def))
-        image: Image = self.build_image()
-        if run_batch or run_server:
-            runner = ContainerRunner(image, self.output_dir)
-            runner.run(run_batch, run_server, from_saved, keep)
+        return self._build_image()
 
     @staticmethod
     def export_conda_env() -> dict:
@@ -253,7 +242,7 @@ def ensure_present(pkg: str):
             ensure_present(package)
         return conda_env
 
-    def build_image(self) -> docker.models.images.Image:
+    def _build_image(self) -> docker.models.images.Image:
         client = docker.from_env()
         dockerfile = textwrap.dedent(
             """
@@ -317,23 +306,26 @@ def client(self):
         return self._client
 
     def run(
-        self, run_batch: bool, run_server: bool, from_saved: bool, keep: bool
+        self,
+        run_batch: bool,
+        host_port: int | None,
+        from_saved: bool,
+        keep: bool,
     ):
         LOGGER.info(f"Running container from image {self.image.short_id}")
         LOGGER.info(f"Image tags: {' '.join(self.image.tags)}")
         command = (
             ["python", "execute.py"]
             + (["--batch"] if run_batch else [])
-            + (["--server"] if run_server else [])
+            + (["--server"] if host_port is not None else [])
             + (["--from-saved"] if from_saved else [])
         )
-        container: Container = self.client.containers.run(
-            image=self.image,
-            command=command,
-            ports={"8080": 8080},
-            remove=False,
-            detach=True,
+        run_args = dict(
+            image=self.image, command=command, remove=False, detach=True
         )
+        if host_port is not None:
+            run_args["ports"] = {"8080": host_port}
+        container: Container = self.client.containers.run(**run_args)
         LOGGER.info(f"Waiting for container {container.short_id} to complete.")
         while container.status in {"created", "running"}:
             LOGGER.debug(
@@ -349,7 +341,7 @@ def run(
             )
             self.extract_output_from_container(container)
             LOGGER.info(f"Results copied.")
-        if not run_server and not keep:
+        if host_port is None and not keep:
             LOGGER.info(f"Removing container {container.short_id}...")
             container.remove(force=True)
             LOGGER.info(f"Container {container.short_id} removed.")

xcengine/util.py

@@ -1,4 +1,4 @@
-# Copyright (c) 2024 by Brockmann Consult GmbH
+# Copyright (c) 2024-2025 by Brockmann Consult GmbH
 # Permissions are hereby granted under the terms of the MIT License:
 # https://opensource.org/licenses/MIT.
 from collections import namedtuple

xcengine/wrapper.py

@@ -1,6 +1,6 @@
 #!/usr/bin/env python3
 
-# Copyright (c) 2024 by Brockmann Consult GmbH
+# Copyright (c) 2024-2025 by Brockmann Consult GmbH
 # Permissions are hereby granted under the terms of the MIT License:
 # https://opensource.org/licenses/MIT.
 
@@ -25,10 +25,13 @@ def __xce_set_params():
     )
     globals().update(params.read_params_combined(sys.argv))
 
+
 if "XC_USER_CODE_PATH" in os.environ:
     __user_code_path = pathlib.Path(os.environ["XC_USER_CODE_PATH"])
 else:
-    __user_code_path = pathlib.Path(__file__).with_name("user_code.py").resolve()
+    __user_code_path = (
+        pathlib.Path(__file__).with_name("user_code.py").resolve()
+    )
 with __user_code_path.open() as fh:
     user_code = fh.read()