Skip to content

fix(local): detect docker compose v2+ when version has no 'v' prefix - #6231

Merged
mohamedzeidan2021 merged 1 commit into
aws:masterfrom
mohamedzeidan2021:fix/docker-compose-version-no-v-prefix-4137
Sep 1, 2026
Merged

fix(local): detect docker compose v2+ when version has no 'v' prefix#6231
mohamedzeidan2021 merged 1 commit into
aws:masterfrom
mohamedzeidan2021:fix/docker-compose-version-no-v-prefix-4137

Conversation

@mohamedzeidan2021

Copy link
Copy Markdown
Collaborator

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

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

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 aws#4137
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown

Claude finished @mohamedzeidan2021's task in 1m 0s —— View job


PR Review — Docker Compose version detection fix

  • Read PR diff and changed files
  • Verify regex correctness (v2+ accept, v1 reject, no-'v'-prefix)
  • Check test coverage across the three v3 copies

Verdict: ✅ Looks good

The fix is correct, minimal, and consistently applied. Changing v(\d+)version\s+v?(\d+) in all three _get_compose_cmd_prefix copies resolves the real bug in #4137:

  • Old regex v(\d+) never matched "Docker Compose version 2.22.0" — there's no v immediately followed by a digit (the v in "version" is followed by e), so brew-installed Compose was wrongly rejected. ✔️ diagnosis is accurate.
  • New regex anchors on the literal version keyword and makes the v optional, so it correctly captures the major version in both "version v2.20.0" (→ 2) and "version 2.22.0" (→ 2).
  • v1 is still rejected: "docker-compose version 1.29.2" → captures 1, and int("1") >= 2 is False, so it falls through to the docker-compose CLI path. Confirmed by the existing v1 tests in sagemaker-train/tests/unit/train/local/test_local_container.py (..._v1_falls_through, ..._v1_no_standalone_raises).
  • Anchoring on version\s+ also tightens things vs. the old loose match, guarding against a stray v<digit> elsewhere in the output.

Tests

Regression tests for the no-'v'-prefix format were added to all three test suites (test_image.py, modules test_local_container.py, train test_local_container.py), each asserting ["docker", "compose"]. Good coverage matching the source change.

Minor (optional, non-blocking)

  • The core copies (sagemaker-core/.../local/image.py and .../modules/local_core/local_container.py) don't have an explicit v1-rejection test with the new "...version 1.x" format the way the train copy does. Not introduced by this PR — the train suite already covers the v1 semantics — but adding parallel v1 tests to the core suites would fully lock in the "≥ 2" branch there too. Not required to merge.

Nothing else stood out — no correctness, security, or backward-compat concerns. The change is a pure bug fix to an internal detection helper (_get_compose_cmd_prefix, private API), so no public interface impact.
· fix/docker-compose-version-no-v-prefix-4137

@mohamedzeidan2021
mohamedzeidan2021 merged commit ef82992 into aws:master Sep 1, 2026
22 of 28 checks passed
@mohamedzeidan2021
mohamedzeidan2021 deleted the fix/docker-compose-version-no-v-prefix-4137 branch September 1, 2026 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docker compose v2

2 participants