fix: use CONTAINER_HOST for Podman - #1233
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughContainer host values are stored and retrieved per runtime, with Docker fallback support for legacy entries. Command wrapping exports Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
lib/view/page/container/container.dart (1)
396-399: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename the Docker-specific host identifiers.
The setting now supports both runtimes, but
_SettingsMenuItems.editDockerHostand_onSaveDockerHostinlib/view/page/container/actions.dart, Line [140], still encode Docker-only semantics. Rename them to runtime-neutral names such aseditContainerHostand_onSaveContainerHost.🤖 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 `@lib/view/page/container/container.dart` around lines 396 - 399, Rename the Docker-specific symbols _SettingsMenuItems.editDockerHost and _onSaveDockerHost in the container settings flow to runtime-neutral names such as editContainerHost and _onSaveContainerHost, and update all references and callbacks accordingly. Preserve the existing behavior for both Podman and Docker host settings.
🤖 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.
Inline comments:
In `@lib/data/provider/container.dart`:
- Around line 467-474: Update the host lookup in _wrap to persist and retrieve
the container host separately for each ContainerType, rather than using only
hostId; ensure Docker and Podman cannot reuse each other’s URI while preserving
the corresponding DOCKER_HOST or CONTAINER_HOST environment variable behavior.
In `@lib/view/page/container/actions.dart`:
- Around line 121-133: Update the Podman branch of the hint in the host-edit
dialog around _onSaveDockerHost to use the rootless socket path, such as
$XDG_RUNTIME_DIR/podman/podman.sock or /run/user/<uid>/podman/podman.sock, while
preserving the existing Docker hint.
---
Nitpick comments:
In `@lib/view/page/container/container.dart`:
- Around line 396-399: Rename the Docker-specific symbols
_SettingsMenuItems.editDockerHost and _onSaveDockerHost in the container
settings flow to runtime-neutral names such as editContainerHost and
_onSaveContainerHost, and update all references and callbacks accordingly.
Preserve the existing behavior for both Podman and Docker host settings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1320bbbc-551e-4545-a97f-79f79a01c9b9
📒 Files selected for processing (3)
lib/data/provider/container.dartlib/view/page/container/actions.dartlib/view/page/container/container.dart
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@lib/data/store/server.dart`:
- Around line 63-66: Update the server ID migration flow around dockerHost to
remove the old typed Docker container entry before storing it under newId. Use
the container API’s typed delete operation for ContainerType.docker and oldId,
while preserving the existing fetch and put behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7cbec3d9-a1bb-4b09-b0bd-b4405c06c296
📒 Files selected for processing (6)
lib/data/provider/container.dartlib/data/store/container.dartlib/data/store/server.dartlib/view/page/container/actions.dartlib/view/page/container/container.dartlib/view/page/container/types.dart
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/view/page/container/container.dart
- lib/data/provider/container.dart
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@lib/data/store/container.dart`:
- Around line 25-27: The typed Docker host removal in ContainerStore.removeHost
must delete both the runtime-scoped key and the legacy id key; update
lib/data/store/container.dart lines 25-27 accordingly. During ID migration,
update the migration flow in lib/data/store/server.dart lines 63-66 to remove
the legacy key before storing the host under newId.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1ab33212-9510-4a9e-b149-8c1b77d50231
📒 Files selected for processing (2)
lib/data/store/container.dartlib/data/store/server.dart
Summary
Validation
Related to #1232.
Summary by CodeRabbit
CONTAINER_HOSTfor Podman andDOCKER_HOSTfor Docker.