From cf3c2f7bdfa57637367fbc894b5e4cf3a37f14be Mon Sep 17 00:00:00 2001 From: kapilkd13 Date: Sat, 22 Jul 2017 18:43:23 +0530 Subject: [PATCH 1/2] adding warning when default docker container is used on Windows --- cwltool/draft2tool.py | 15 ++++++++++++++- cwltool/main.py | 4 ++-- cwltool/utils.py | 1 + 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/cwltool/draft2tool.py b/cwltool/draft2tool.py index 13c5bf543..9c96e5526 100644 --- a/cwltool/draft2tool.py +++ b/cwltool/draft2tool.py @@ -28,13 +28,24 @@ _logger_validation_warnings, compute_checksums, normalizeFilesDirs, shortname, uniquename) from .stdfsaccess import StdFsAccess -from .utils import aslist, docker_windows_path_adjust, convert_pathsep_to_unix +from .utils import aslist, docker_windows_path_adjust, convert_pathsep_to_unix, windows_default_container_id, onWindows from six.moves import map ACCEPTLIST_EN_STRICT_RE = re.compile(r"^[a-zA-Z0-9._+-]+$") ACCEPTLIST_EN_RELAXED_RE = re.compile(r".*") # Accept anything ACCEPTLIST_RE = ACCEPTLIST_EN_STRICT_RE +DEFAULT_CONTAINER_MSG="""We are on Microsoft Windows and not all components of this CWL description have a +container specified. This means that these steps will be executed in the default container, +which is %s. +Note, this could affect portability if this CWL description relies on non-POSIX features +or commands in this container. For best results add the following to your CWL +description's hints section: + +hints: + DockerRequirement: + dockerPull: %s +""" _logger = logging.getLogger("cwltool") @@ -189,6 +200,8 @@ def makeJobRunner(self, use_container=True): # type: (Optional[bool]) -> JobBas "dockerPull": default_container }) dockerReq = self.requirements[0] + if default_container == windows_default_container_id and use_container and onWindows(): + _logger.warning(DEFAULT_CONTAINER_MSG%(windows_default_container_id, windows_default_container_id)) if dockerReq and use_container: return DockerCommandLineJob() diff --git a/cwltool/main.py b/cwltool/main.py index 04f74b5c6..9eaeab136 100755 --- a/cwltool/main.py +++ b/cwltool/main.py @@ -39,7 +39,7 @@ from .software_requirements import DependenciesConfiguration, get_container_from_software_requirements, SOFTWARE_REQUIREMENTS_ENABLED from .stdfsaccess import StdFsAccess from .update import ALLUPDATES, UPDATES -from .utils import onWindows +from .utils import onWindows, windows_default_container_id from ruamel.yaml.comments import Comment, CommentedSeq, CommentedMap @@ -712,7 +712,7 @@ def main(argsl=None, # type: List[str] # If On windows platform, A default Docker Container is Used if not explicitely provided by user if onWindows() and not args.default_container: # This docker image is a minimal alpine image with bash installed(size 6 mb). source: https://github.com/frol/docker-alpine-bash - args.default_container = "frolvlad/alpine-bash" + args.default_container = windows_default_container_id # If caller provided custom arguments, it may be not every expected # option is set, so fill in no-op defaults to avoid crashing when diff --git a/cwltool/utils.py b/cwltool/utils.py index b78359e51..abddfcfef 100644 --- a/cwltool/utils.py +++ b/cwltool/utils.py @@ -10,6 +10,7 @@ from six.moves import zip_longest from typing import Any,Callable, Dict, List, Tuple, Text, Union +windows_default_container_id = "frolvlad/alpine-bash" def aslist(l): # type: (Any) -> List[Any] if isinstance(l, list): From 7b5f82a5dec6827cfe8f969a71b74ae2c433eed7 Mon Sep 17 00:00:00 2001 From: kapilkd13 Date: Wed, 26 Jul 2017 12:30:36 +0530 Subject: [PATCH 2/2] adding test to check that warning is printed when default Docker container is used on windows --- tests/test_docker_warning.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 tests/test_docker_warning.py diff --git a/tests/test_docker_warning.py b/tests/test_docker_warning.py new file mode 100644 index 000000000..8040b3808 --- /dev/null +++ b/tests/test_docker_warning.py @@ -0,0 +1,25 @@ +from __future__ import absolute_import +import unittest +from mock import mock +from cwltool.utils import windows_default_container_id +from cwltool.draft2tool import DEFAULT_CONTAINER_MSG, CommandLineTool + + +class TestDefaultDockerWarning(unittest.TestCase): + + # Test to check warning when default docker Container is used on Windows + @mock.patch("cwltool.draft2tool.onWindows",return_value = True) + @mock.patch("cwltool.draft2tool._logger") + def test_default_docker_warning(self,mock_logger,mock_windows): + + class TestCommandLineTool(CommandLineTool): + def __init__(self, **kwargs): + self.requirements=[] + self.hints=[] + + def find_default_container(args, builder): + return windows_default_container_id + + TestObject = TestCommandLineTool() + TestObject.makeJobRunner() + mock_logger.warning.assert_called_with(DEFAULT_CONTAINER_MSG%(windows_default_container_id, windows_default_container_id))