refactor(compose): use ContainerRuntime for compose detection ordering#282
Merged
Conversation
…on ordering NewComposeHelper now queries dockerHelper.GetRuntime().Name() to determine the compose detection order — Podman runtimes try podman compose first, while Docker/nerdctl runtimes try docker compose V2 first. Removes hardcoded podmanCmd and composeArg constants. tryPodmanCompose now accepts the docker command as a parameter instead of using a hardcoded "podman" string, using dockerHelper.DockerCommand consistently throughout.
The non-Podman detection path now passes "podman" (hardcoded) to the fallback detector instead of re-using dockerCmd, preserving the original behavior of independently probing for podman in PATH. Renames tryPodmanCompose to tryComposeSubcommand to reflect that the function generically probes `<cmd> compose` for any docker command.
✅ Deploy Preview for devsydev canceled.
|
📝 WalkthroughWalkthroughThe pull request refactors compose command detection in ChangesCompose command detection refactor
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Resolves goconst lint violations flagged by golangci-lint.
skevetter
marked this pull request as ready for review
May 14, 2026 10:01
skevetter
enabled auto-merge (squash)
May 14, 2026 10:02
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
pkg/compose/helper.go (2)
93-93: 💤 Low valueError message could mention docker-compose V1.
The fallback logic includes
tryDockerComposeV1, but the error message only mentions "docker compose or podman compose". Consider updating it to reflect all three options (e.g., "docker compose, podman compose, or docker-compose not installed").🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/compose/helper.go` at line 93, The error return in the function that falls back through tryDockerCompose, tryPodmanCompose, and tryDockerComposeV1 currently says "docker compose or podman compose not installed"; update that fmt.Errorf message to include docker-compose V1 as well (e.g., "docker compose, podman compose, or docker-compose not installed") so the error accurately reflects all three attempted options; locate the failing return in pkg/compose/helper.go near the fallback logic that calls tryDockerCompose, tryPodmanCompose, and tryDockerComposeV1 and replace the error string accordingly.
72-84: ⚖️ Poor tradeoffRedundant detection in Podman runtime branch.
When
dockerHelper.GetRuntime().Name()returnsRuntimePodman, the first two detectors both invoke compose subcommand checks ondockerCmd:
tryComposeSubcommand(dockerCmd)— checks ifdockerCmdbinary exists, validates compose availability, and parses versiontryDockerComposeV2(dockerCmd)— validates compose availability without checking binary existence or parsing versionIf
dockerCmdis"podman"(the typical case for Podman runtimes), both detectors probepodman compose. The second detector only succeeds if the first fails due to version parsing errors, which could mask parsing issues rather than surfacing them.Consider revising the Podman branch to avoid redundant probing. For example, remove the second detector entirely, or replace it with a fallback to a different command (e.g.,
tryDockerComposeV2("docker")):♻️ Proposed fix to eliminate redundancy
Option 1: Remove redundant detector
if dockerHelper.GetRuntime().Name() == docker.RuntimePodman { detectors = []tryFunc{ func() (*ComposeHelper, error) { return tryComposeSubcommand(dockerCmd) }, - func() (*ComposeHelper, error) { return tryDockerComposeV2(dockerCmd) }, tryDockerComposeV1, }Option 2: Use explicit docker fallback
if dockerHelper.GetRuntime().Name() == docker.RuntimePodman { detectors = []tryFunc{ func() (*ComposeHelper, error) { return tryComposeSubcommand(dockerCmd) }, - func() (*ComposeHelper, error) { return tryDockerComposeV2(dockerCmd) }, + func() (*ComposeHelper, error) { return tryDockerComposeV2("docker") }, tryDockerComposeV1, }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/compose/helper.go` around lines 72 - 84, When runtime is Podman (dockerHelper.GetRuntime().Name() == RuntimePodman) the detectors slice redundantly calls tryComposeSubcommand(dockerCmd) and then tryDockerComposeV2(dockerCmd) which both probe the same podman compose; remove the middle redundant detector so the Podman branch uses only tryComposeSubcommand(dockerCmd) followed by tryDockerComposeV1, or if you prefer an explicit Docker fallback replace the middle entry with func() (*ComposeHelper, error) { return tryDockerComposeV2("docker") } — update the detectors slice in the Podman branch accordingly (referencing dockerCmd, tryComposeSubcommand, tryDockerComposeV2, tryDockerComposeV1 and RuntimePodman).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@pkg/compose/helper.go`:
- Line 93: The error return in the function that falls back through
tryDockerCompose, tryPodmanCompose, and tryDockerComposeV1 currently says
"docker compose or podman compose not installed"; update that fmt.Errorf message
to include docker-compose V1 as well (e.g., "docker compose, podman compose, or
docker-compose not installed") so the error accurately reflects all three
attempted options; locate the failing return in pkg/compose/helper.go near the
fallback logic that calls tryDockerCompose, tryPodmanCompose, and
tryDockerComposeV1 and replace the error string accordingly.
- Around line 72-84: When runtime is Podman (dockerHelper.GetRuntime().Name() ==
RuntimePodman) the detectors slice redundantly calls
tryComposeSubcommand(dockerCmd) and then tryDockerComposeV2(dockerCmd) which
both probe the same podman compose; remove the middle redundant detector so the
Podman branch uses only tryComposeSubcommand(dockerCmd) followed by
tryDockerComposeV1, or if you prefer an explicit Docker fallback replace the
middle entry with func() (*ComposeHelper, error) { return
tryDockerComposeV2("docker") } — update the detectors slice in the Podman branch
accordingly (referencing dockerCmd, tryComposeSubcommand, tryDockerComposeV2,
tryDockerComposeV1 and RuntimePodman).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 09e70735-4049-4f52-9862-f0465d992c8e
📒 Files selected for processing (2)
pkg/compose/helper.gopkg/compose/helper_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
NewComposeHelper()to usedockerHelper.GetRuntime().Name()to determine compose detection order — Podman runtimes trypodman composefirst, Docker/nerdctl runtimes trydocker composeV2 firstpodmanCmdandcomposeArgpackage-level constantstryPodmanCompose()totryComposeSubcommand()— now accepts docker command as parameter for generalized compose subcommand detectionpodmanin PATHstubRuntimetest double (22 total tests pass)Closes DEVSY-099
Summary by CodeRabbit
Refactor
Tests