Do not require docker save

common-workflow-language · Oct 3, 2023 · 0a4fa3a · 0a4fa3a
1 parent 823b687
commit 0a4fa3a
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 24 deletions.
diff --git a/README.rst b/README.rst
@@ -49,15 +49,15 @@ the software container images in Docker format.
 
 .. code:: bash
 
-   cwl-docker-extract DIRECTORY path_to_my_workflow.cwl
+   cwl-docker-extract path_to_my_workflow.cwl
 
 Or you can use the Singularity software container engine to download and
 save the software container images and convert them to the Singularity
 format at the same time.
 
 .. code:: bash
 
-   cwl-docker-extract --singularity DIRECTORY path_to_my_workflow.cwl
+   cwl-docker-extract --singularity --dir DIRECTORY path_to_my_workflow.cwl
 
 
 Print all referenced software packages

diff --git a/cwl_utils/docker_extract.py b/cwl_utils/docker_extract.py
@@ -21,10 +21,10 @@ def arg_parser() -> argparse.ArgumentParser:
         description="Save container images specified in a CWL document (Workflow or CommandLineTool). "
         "For CWL Workflows, all steps will also be searched (recursively)."
     )
-    parser.add_argument("dir", help="Directory in which to save images")
     parser.add_argument(
         "input", help="Input CWL document (CWL Workflow or CWL CommandLineTool)"
     )
+    parser.add_argument("--dir", help="Directory in which to save images")
     parser.add_argument(
         "-s",
         "--singularity",
@@ -45,7 +45,12 @@ def arg_parser() -> argparse.ArgumentParser:
 
 def run(args: argparse.Namespace) -> List[cwl.DockerRequirement]:
     """Extract the docker reqs and download them using Singularity or Docker."""
-    os.makedirs(args.dir, exist_ok=True)
+    if args.singularity and not args.dir:
+        print("Error! Must specify --dir if using --singularity")
+        sys.exit(1)
+
+    if args.dir:
+        os.makedirs(args.dir, exist_ok=True)
 
     top = cwl.load_document_by_uri(args.input)
     reqs: List[cwl.DockerRequirement] = []

diff --git a/cwl_utils/image_puller.py b/cwl_utils/image_puller.py
@@ -5,7 +5,7 @@
 import subprocess  # nosec
 from abc import ABC, abstractmethod
 from pathlib import Path
-from typing import List, Union
+from typing import List, Optional, Union
 
 from .singularity import get_version as get_singularity_version
 from .singularity import is_version_2_6 as is_singularity_version_2_6
@@ -17,7 +17,11 @@
 
 class ImagePuller(ABC):
     def __init__(
-        self, req: str, save_directory: Union[str, Path], cmd: str, force_pull: bool
+        self,
+        req: str,
+        save_directory: Optional[Union[str, Path]],
+        cmd: str,
+        force_pull: bool,
     ) -> None:
         """Create an ImagePuller."""
         self.req = req
@@ -62,22 +66,24 @@ def generate_udocker_loading_command(self) -> str:
 
     def save_docker_image(self) -> None:
         """Download and save the software container image to disk as a docker tarball."""
-        _LOGGER.info(f"Pulling {self.req} with Docker...")
+        _LOGGER.info(f"Pulling {self.req} with {self.cmd}...")
         cmd_pull = [self.cmd, "pull", self.req]
         ImagePuller._run_command_pull(cmd_pull)
-        dest = os.path.join(self.save_directory, self.get_image_name())
-        if self.force_pull:
-            os.remove(dest)
-        cmd_save = [
-            self.cmd,
-            "save",
-            "-o",
-            dest,
-            self.req,
-        ]
-        subprocess.run(cmd_save, check=True)  # nosec
-        _LOGGER.info(f"Image successfully pulled: {dest!r}.")
-        print(self.generate_udocker_loading_command())
+        _LOGGER.info(f"Image successfully pulled: {self.req}")
+        if self.save_directory:
+            dest = os.path.join(self.save_directory, self.get_image_name())
+            if self.save_directory and self.force_pull:
+                os.remove(dest)
+            cmd_save = [
+                self.cmd,
+                "save",
+                "-o",
+                dest,
+                self.req,
+            ]
+            subprocess.run(cmd_save, check=True)  # nosec
+            _LOGGER.info(f"Image successfully saved: {dest!r}.")
+            print(self.generate_udocker_loading_command())
 
 
 class SingularityImagePuller(ImagePuller):
@@ -103,8 +109,11 @@ def get_image_name(self) -> str:
 
     def save_docker_image(self) -> None:
         """Pull down the Docker software container image and save it in the Singularity image format."""
+        save_directory: Union[str, Path]
+        if self.save_directory:
+            save_directory = self.save_directory
         if (
-            os.path.exists(os.path.join(self.save_directory, self.get_image_name()))
+            os.path.exists(os.path.join(save_directory, self.get_image_name()))
             and not self.force_pull
         ):
             _LOGGER.info(f"Already cached {self.req} with Singularity.")
@@ -119,11 +128,11 @@ def save_docker_image(self) -> None:
         cmd_pull.extend(
             [
                 "--name",
-                os.path.join(self.save_directory, self.get_image_name()),
+                os.path.join(save_directory, self.get_image_name()),
                 f"docker://{self.req}",
             ]
         )
         ImagePuller._run_command_pull(cmd_pull)
         _LOGGER.info(
-            f"Image successfully pulled: {self.save_directory}/{self.get_image_name()}"
+            f"Image successfully pulled: {save_directory}/{self.get_image_name()}"
         )
diff --git a/tests/test_docker_extract.py b/tests/test_docker_extract.py
@@ -23,7 +23,7 @@
 def test_container_extraction(target: str, engine: str, tmp_path: Path) -> None:
     """Test container extraction tool."""
 
-    args = [str(tmp_path), get_data(target), "--container-engine", engine]
+    args = ["--dir", str(tmp_path), get_data(target), "--container-engine", engine]
     if engine == "singularity":
         args.append("--singularity")
     reqs = run(arg_parser().parse_args(args))
@@ -43,6 +43,7 @@ def test_container_extraction_force(engine: str, tmp_path: Path) -> None:
     """Test force pull container extraction."""
 
     args = [
+        "--dir",
         str(tmp_path),
         get_data("testdata/md5sum.cwl"),
         "--container-engine",
@@ -54,6 +55,7 @@ def test_container_extraction_force(engine: str, tmp_path: Path) -> None:
     assert len(reqs) == 1
     assert len(list(tmp_path.iterdir())) == 1
     args = [
+        "--dir",
         str(tmp_path),
         get_data("testdata/md5sum.cwl"),
         "--container-engine",
@@ -81,6 +83,7 @@ def test_container_extraction_no_dockerPull(
     """Test container extraction tool when dockerPull is missing."""
 
     args = [
+        "--dir",
         str(tmp_path),
         get_data("testdata/debian_image_id.cwl"),
         "--container-engine",
@@ -113,6 +116,7 @@ def test_container_extraction_embedded_step(engine: str, tmp_path: Path) -> None
     """Test container extraction tool."""
 
     args = [
+        "--dir",
         str(tmp_path),
         get_data("testdata/workflows/count-lines16-wf.cwl"),
         "--container-engine",