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

github-actionsBot 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 regexv(\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:masterSep 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 freeto 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

@mohamedzeidan2021@lucasjia-aws
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
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.
Fixesaws#4137
@github-actions

github-actionsBot 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 regexv(\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:masterSep 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 freeto 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

@mohamedzeidan2021@lucasjia-aws
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
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.
Fixesaws#4137
@github-actions

github-actionsBot 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 regexv(\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:masterSep 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 freeto 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

@mohamedzeidan2021@lucasjia-aws
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
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.
Fixesaws#4137
@github-actions

github-actionsBot 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 regexv(\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:masterSep 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 freeto 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

@mohamedzeidan2021@lucasjia-aws
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
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.
Fixesaws#4137
@github-actions

github-actionsBot 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 regexv(\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:masterSep 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 freeto 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

@mohamedzeidan2021@lucasjia-aws
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
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.
Fixesaws#4137
@github-actions

github-actionsBot 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 regexv(\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:masterSep 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 freeto 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

@mohamedzeidan2021@lucasjia-aws
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
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.
Fixesaws#4137
@github-actions

github-actionsBot 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 regexv(\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:masterSep 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 freeto 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

@mohamedzeidan2021@lucasjia-aws
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
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.
Fixesaws#4137
@github-actions

github-actionsBot 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 regexv(\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:masterSep 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 freeto 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

@mohamedzeidan2021@lucasjia-aws