From 71de589b86bbdb36c79c3120cf7bac10aafa3a99 Mon Sep 17 00:00:00 2001 From: Mohamed Zeidan Date: Mon, 31 Aug 2026 20:04:36 -0700 Subject: [PATCH] fix(local): detect docker compose v2+ when version has no 'v' prefix Docker Compose installed via Homebrew reports its version without a leading 'v' (e.g. "Docker Compose version 2.22.0"). The detection regex `v(\d+)` required a literal 'v' before the digits, so `_get_compose_cmd_prefix` failed to recognize a valid Compose v2+ plugin in that case. Change the regex to `version\s+v?(\d+)`, making the 'v' optional and anchoring to the "version" keyword so a stray number elsewhere in the output can't cause a false positive. v1 is still correctly rejected. Applied to all three v3 copies (core local image, core modules local container, train local container) with regression tests for the no-'v'-prefix format. Fixes #4137 --- .../src/sagemaker/core/local/image.py | 2 +- .../modules/local_core/local_container.py | 2 +- sagemaker-core/tests/unit/local/test_image.py | 12 ++++++++ .../local_core/test_local_container.py | 28 +++++++++++++++++++ .../sagemaker/train/local/local_container.py | 2 +- .../unit/train/local/test_local_container.py | 13 +++++++++ 6 files changed, 56 insertions(+), 3 deletions(-) diff --git a/sagemaker-core/src/sagemaker/core/local/image.py b/sagemaker-core/src/sagemaker/core/local/image.py index 6da0db50fb..a0a31d8b4c 100644 --- a/sagemaker-core/src/sagemaker/core/local/image.py +++ b/sagemaker-core/src/sagemaker/core/local/image.py @@ -163,7 +163,7 @@ def _get_compose_cmd_prefix(): ) if output: - match = re.search(r"v(\d+)", output.strip()) + match = re.search(r"version\s+v?(\d+)", output.strip()) if match and int(match.group(1)) >= 2: logger.info("'Docker Compose' found using Docker CLI.") compose_cmd_prefix.extend(["docker", "compose"]) diff --git a/sagemaker-core/src/sagemaker/core/modules/local_core/local_container.py b/sagemaker-core/src/sagemaker/core/modules/local_core/local_container.py index 06de1cf6ca..ede32c5eae 100644 --- a/sagemaker-core/src/sagemaker/core/modules/local_core/local_container.py +++ b/sagemaker-core/src/sagemaker/core/modules/local_core/local_container.py @@ -618,7 +618,7 @@ def _get_compose_cmd_prefix(self) -> List[str]: ) if output: - match = re.search(r"v(\d+)", output.strip()) + match = re.search(r"version\s+v?(\d+)", output.strip()) if match and int(match.group(1)) >= 2: logger.info("'Docker Compose' found using Docker CLI.") compose_cmd_prefix.extend(["docker", "compose"]) diff --git a/sagemaker-core/tests/unit/local/test_image.py b/sagemaker-core/tests/unit/local/test_image.py index 7a7962c19e..714bed2cc1 100644 --- a/sagemaker-core/tests/unit/local/test_image.py +++ b/sagemaker-core/tests/unit/local/test_image.py @@ -401,6 +401,18 @@ def test_get_compose_cmd_prefix_docker_compose_v2(self, mock_check_output): assert result == ["docker", "compose"] + @patch("subprocess.check_output") + def test_get_compose_cmd_prefix_docker_compose_v2_no_v_prefix(self, mock_check_output): + """Docker Compose installed via brew reports the version without a 'v' prefix. + + Regression test for https://github.com/aws/sagemaker-python-sdk/issues/4137. + """ + mock_check_output.return_value = "Docker Compose version 2.22.0" + + result = _SageMakerContainer._get_compose_cmd_prefix() + + assert result == ["docker", "compose"] + @patch("shutil.which") @patch("subprocess.check_output") def test_get_compose_cmd_prefix_docker_compose_cli(self, mock_check_output, mock_which): diff --git a/sagemaker-core/tests/unit/modules/local_core/test_local_container.py b/sagemaker-core/tests/unit/modules/local_core/test_local_container.py index a4c137484d..6fc15351d8 100644 --- a/sagemaker-core/tests/unit/modules/local_core/test_local_container.py +++ b/sagemaker-core/tests/unit/modules/local_core/test_local_container.py @@ -585,6 +585,34 @@ def test_get_compose_cmd_prefix_docker_compose_v2( assert result == ["docker", "compose"] + @patch("sagemaker.core.modules.local_core.local_container.subprocess.check_output") + def test_get_compose_cmd_prefix_docker_compose_v2_no_v_prefix( + self, mock_check_output, mock_session, basic_channel + ): + """Brew-installed Docker Compose reports the version without a 'v' prefix. + + Regression test for https://github.com/aws/sagemaker-python-sdk/issues/4137. + """ + container = _LocalContainer( + training_job_name="test-job", + instance_type="local", + instance_count=1, + image="test-image:latest", + container_root="/tmp/test", + input_data_config=[basic_channel], + environment={}, + hyper_parameters={}, + container_entrypoint=[], + container_arguments=[], + sagemaker_session=mock_session, + ) + + mock_check_output.return_value = "Docker Compose version 2.22.0" + + result = container._get_compose_cmd_prefix() + + assert result == ["docker", "compose"] + @patch("sagemaker.core.modules.local_core.local_container.subprocess.check_output") @patch("sagemaker.core.modules.local_core.local_container.shutil.which") def test_get_compose_cmd_prefix_docker_compose_standalone( diff --git a/sagemaker-train/src/sagemaker/train/local/local_container.py b/sagemaker-train/src/sagemaker/train/local/local_container.py index 26f9a62e9f..558a95ffa4 100644 --- a/sagemaker-train/src/sagemaker/train/local/local_container.py +++ b/sagemaker-train/src/sagemaker/train/local/local_container.py @@ -626,7 +626,7 @@ def _get_compose_cmd_prefix(self) -> List[str]: ) if output: - match = re.search(r"v(\d+)", output.strip()) + match = re.search(r"version\s+v?(\d+)", output.strip()) if match and int(match.group(1)) >= 2: logger.info("'Docker Compose' found using Docker CLI.") compose_cmd_prefix.extend(["docker", "compose"]) diff --git a/sagemaker-train/tests/unit/train/local/test_local_container.py b/sagemaker-train/tests/unit/train/local/test_local_container.py index 7393b6c6f0..f50aa9a13c 100644 --- a/sagemaker-train/tests/unit/train/local/test_local_container.py +++ b/sagemaker-train/tests/unit/train/local/test_local_container.py @@ -128,6 +128,19 @@ def test_get_compose_cmd_prefix_with_docker_compose_v2(self, mock_check_output, result = container._get_compose_cmd_prefix() assert result == ["docker", "compose"] + @patch("sagemaker.train.local.local_container.subprocess.check_output") + def test_get_compose_cmd_prefix_with_docker_compose_v2_no_v_prefix( + self, mock_check_output, _basic_channel + ): + """Brew-installed Docker Compose reports the version without a 'v' prefix. + + Regression test for https://github.com/aws/sagemaker-python-sdk/issues/4137. + """ + container = _make_container(_basic_channel) + mock_check_output.return_value = "Docker Compose version 2.22.0" + result = container._get_compose_cmd_prefix() + assert result == ["docker", "compose"] + @patch("sagemaker.train.local.local_container.subprocess.check_output") def test_get_compose_cmd_prefix_with_docker_compose_v5(self, mock_check_output, _basic_channel): """Docker Compose v5 should be accepted."""