Move labctl.py to scripts folder - #189
Conversation
WalkthroughReferences to Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Operator
participant TaskRunner as Task runner
participant Labctl as scripts/labctl.py
participant Stacks as repo_root/docker
participant Docker as Docker Engine
Operator->>TaskRunner: task docker:apply / task docker:command
TaskRunner->>Labctl: scripts/labctl.py service <op> <service>
Note right of Labctl: docker_stacks_dir -> repo_root/docker
Labctl->>Stacks: read stack definitions
Labctl->>Docker: run compose config/apply/update/pull/stop
Docker-->>Operator: result/status
sequenceDiagram
autonumber
participant Controller as scripts/task-mcp/server.py
participant Labctl as scripts/labctl.py
participant Docker as Docker Engine
Controller->>Labctl: python scripts/labctl.py service <operation> <service>
Labctl->>Docker: execute requested operation
Docker-->>Controller: status/exit code
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docker/backup.sh (1)
8-11: Fix stop when no containers running (set -e causes hard fail)docker stop with an empty arg list exits non‑zero and aborts the script. Use xargs -r to be ShellCheck‑clean and robust.
stop_all_containers() { - # shellcheck disable=SC2046 - docker stop $(docker ps -q) + docker ps -q | xargs -r docker stop }
🧹 Nitpick comments (6)
scripts/labctl.py (1)
22-22: Prefer resolve() over absolute() for robustness.Using resolve() handles symlinks and yields a canonical absolute path.
Apply this diff:
-docker_stacks_dir: Path = (Path(__file__).parent.parent / "docker").absolute() +docker_stacks_dir: Path = (Path(__file__).resolve().parent.parent / "docker").resolve()docker/backup.sh (3)
13-15: Switch to task docker:apply looks good; add a guard for Task availabilityIf task isn’t installed, the script will fail mid-backup. Add a quick check with a helpful error.
start_selected_containers() { - task docker:apply + if ! command -v task >/dev/null 2>&1; then + echo "Error: 'task' is not installed. See https://taskfile.dev/#/installation" >&2 + return 127 + fi + task docker:apply }
25-28: Ensure containers are restarted even on failureIf compression fails, containers remain stopped. Use a trap to always restart selected containers.
+trap 'start_selected_containers' EXIT stop_all_containers #kopia_backup_docker_volumes compress_docker_volumes -start_selected_containers +# start_selected_containers is called by trap
17-19: Non-interactive execDrop -t (and likely -i) for non-interactive runs to avoid TTY issues when used in automation.
- docker exec -it kopia-nas /usr/bin/kopia --config-file=/app/config/repository.config snapshot create /sources/nest/docker-volumes + docker exec kopia-nas /usr/bin/kopia --config-file=/app/config/repository.config snapshot create /sources/nest/docker-volumesCLAUDE.md (1)
147-147: Minor grammar/style nitConsider “Python for service management via scripts/labctl.py” (drop “the … tool”) for brevity.
.claude/commands/implement-container-deployment.md (1)
25-26: Make commands copy-pasteable from repo rootPrefix with ./ so users don’t depend on current working directory PATH or executable resolution.
Apply this diff:
- - Validate the compose file with `scripts/labctl.py service config <category>/<application>` and fix any errors or warnings. + - Validate the compose file with `./scripts/labctl.py service config <category>/<application>` and fix any errors or warnings. - - Pull the container image(s) with the command `scripts/labctl.py service pull <category>/<application>` and verify success. + - Pull the container image(s) with the command `./scripts/labctl.py service pull <category>/<application>` and verify success.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
.claude/commands/implement-container-deployment.md(1 hunks).claude/settings.json(1 hunks)CLAUDE.md(4 hunks)Taskfile.yaml(1 hunks)docker/README.md(1 hunks)docker/Taskfile.docker.yaml(2 hunks)docker/backup.sh(1 hunks)docs/setup.md(1 hunks)docs/web/Taskfile.web.yaml(1 hunks)scripts/labctl.py(1 hunks)scripts/task-mcp/server.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Python code must pass Ruff linting
Files:
scripts/task-mcp/server.pyscripts/labctl.py
**/*.{yml,yaml}
📄 CodeRabbit inference engine (CLAUDE.md)
YAML files must be linted and valid (pre-commit runs YAML linting/validation)
Files:
Taskfile.yamldocker/Taskfile.docker.yamldocs/web/Taskfile.web.yaml
**/*.sh
📄 CodeRabbit inference engine (CLAUDE.md)
Shell scripts must pass ShellCheck (pre-commit enforces shell script validation)
Files:
docker/backup.sh
🧠 Learnings (1)
📚 Learning: 2025-08-30T07:46:23.482Z
Learnt from: CR
PR: bubacoder/infra#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-30T07:46:23.482Z
Learning: Applies to docker/{security,media,storage,monitoring,ai,tools}/**/*.{yml,yaml} : Place and maintain service YAML definitions under the appropriate docker category directory
Applied to files:
CLAUDE.md
🧬 Code graph analysis (7)
scripts/task-mcp/server.py (1)
docker/labctl.py (5)
main(234-261)docker_command(92-135)cmd_service(214-231)cmd_config_apply(189-211)process_services(149-179)
docker/README.md (1)
docker/labctl.py (4)
cmd_config_apply(189-211)main(234-261)process_services(149-179)docker(88-89)
Taskfile.yaml (1)
docker/labctl.py (5)
docker_command(92-135)main(234-261)cmd_config_apply(189-211)cmd_service(214-231)docker(88-89)
docs/web/Taskfile.web.yaml (1)
docs/web/update-docs.py (4)
process(355-365)process_docker_directory(340-353)process_docker_stack_index(253-261)process_docker_compose_file(296-338)
.claude/commands/implement-container-deployment.md (1)
docker/labctl.py (2)
docker_command(92-135)main(234-261)
docker/backup.sh (1)
docker/labctl.py (3)
main(234-261)cmd_config_apply(189-211)docker_command(92-135)
CLAUDE.md (1)
docker/labctl.py (6)
main(234-261)cmd_config_apply(189-211)docker_command(92-135)docker(88-89)process_services(149-179)create_localhost_link(34-52)
🪛 LanguageTool
CLAUDE.md
[grammar] ~147-~147: There might be a mistake here.
Context: ...agement via the scripts/labctl.py tool - GitHub Actions for CI/CD workflows: ...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: check
🔇 Additional comments (12)
scripts/task-mcp/server.py (1)
145-150: Path switched to scripts/labctl.py — looks good.The new path correctly targets the relocated script and preserves the existing invocation pattern with
sys.executable.scripts/labctl.py (1)
25-25: No-op formatting change.Nothing to do here.
docker/README.md (1)
30-30: Consolidated to a single apply command — clear and consistent.docker/Taskfile.docker.yaml (2)
12-12: Path updates to scripts/labctl.py — consistent across tasks.Also applies to: 17-17, 22-22, 35-35
7-35: scripts/labctl.py is executable and no stale docker/labctl.py references remaindocs/setup.md (1)
116-116: Docs aligned to the single entrypoint (task docker:apply).docs/web/Taskfile.web.yaml (1)
7-7: Path swap to scripts/labctl.py is consistent with repo moveLGTM. Validate that commands execute from repo root (as other cmds here assume) so the relative path resolves correctly.
.claude/settings.json (1)
8-9: Allow-list updates align with new locationLooks correct and minimal.
Taskfile.yaml (1)
108-108: Approve: no remaining docker/labctl.py references
All references have been updated toscripts/labctl.py.CLAUDE.md (2)
58-64: Examples updated to scripts/labctl.py — OKExamples cover nested categories and align with tool behavior.
156-169: Docs align with repository conventionThis reflects the learning to keep service YAMLs under docker category directories; good reinforcement.
.claude/commands/implement-container-deployment.md (1)
25-26: LGTM: path update aligns with labctl moveUpdating to scripts/labctl.py for both config and pull is consistent with the repo-wide migration.
739f5bd to
63417bc
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
CLAUDE.md (3)
58-64: Clarify invocation to avoid PATH/shebang surprises; include subcategory in placeholder.Many environments won’t have scripts/ on PATH or executable bit set. Using explicit python3 also communicates the interpreter. Also, the placeholder should reflect optional subcategories.
- scripts/labctl.py service [operation] [category/service-name] + python3 scripts/labctl.py service [operation] [category[/subcategory]/service-name] - scripts/labctl.py service up security/traefik - scripts/labctl.py service restart ai/ollama - scripts/labctl.py service update media/video/jellyfin + python3 scripts/labctl.py service up security/traefik + python3 scripts/labctl.py service restart ai/ollama + python3 scripts/labctl.py service update media/video/jellyfinFollow-up:
- Ensure scripts/labctl.py has a valid shebang and executable bit if you prefer the direct form. Otherwise the python3 form above is safer in docs.
147-147: Minor punctuation for list consistency.Add a terminal period to match surrounding bullets and silence the grammar warning.
- - **Python** for service management via the `scripts/labctl.py` tool + - **Python** for service management via the `scripts/labctl.py` tool.
156-163: Call out required working directory for the relocated script.Make it explicit that labctl must be run from repo root so it can resolve docker/. This prevents “Compose file not found” issues after the move.
-The repository uses a custom Python script (`scripts/labctl.py`) to manage Docker services defined in YAML files: +The repository uses a custom Python script (`scripts/labctl.py`) to manage Docker services defined in YAML files (run it from the repository root so it can locate the `docker/` directory):Verification:
- Confirm scripts/labctl.py computes docker_stacks_dir relative to repo root (not two levels up). If desired, I can open a follow-up PR to add a guard that errors with a clear message when run outside the repo root.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
.claude/commands/implement-container-deployment.md(1 hunks).claude/settings.json(1 hunks)CLAUDE.md(4 hunks)Taskfile.yaml(1 hunks)docker/README.md(1 hunks)docker/Taskfile.docker.yaml(2 hunks)docker/backup.sh(1 hunks)docs/setup.md(1 hunks)docs/web/Taskfile.web.yaml(1 hunks)scripts/labctl.py(1 hunks)scripts/task-mcp/server.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- docker/README.md
🚧 Files skipped from review as they are similar to previous changes (9)
- docker/backup.sh
- .claude/settings.json
- scripts/labctl.py
- scripts/task-mcp/server.py
- docs/setup.md
- docs/web/Taskfile.web.yaml
- Taskfile.yaml
- .claude/commands/implement-container-deployment.md
- docker/Taskfile.docker.yaml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-30T07:46:23.482Z
Learnt from: CR
PR: bubacoder/infra#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-30T07:46:23.482Z
Learning: Applies to docker/{security,media,storage,monitoring,ai,tools}/**/*.{yml,yaml} : Place and maintain service YAML definitions under the appropriate docker category directory
Applied to files:
CLAUDE.md
🧬 Code graph analysis (1)
CLAUDE.md (1)
docker/labctl.py (4)
main(234-261)cmd_config_apply(189-211)docker_command(92-135)docker(88-89)
🪛 LanguageTool
CLAUDE.md
[grammar] ~147-~147: There might be a mistake here.
Context: ...agement via the scripts/labctl.py tool - GitHub Actions for CI/CD workflows: ...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: check
- GitHub Check: build
🔇 Additional comments (1)
CLAUDE.md (1)
185-185: Directory listing looks good and matches our category convention.Retains docker/tools/ alongside security, media, storage, monitoring, ai, aligning with the established layout.
Summary by CodeRabbit
Documentation
Refactor
Chores