Misc fixes & improvements on install script #306
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the Linux Docker environment installer and the prod.local compose configuration to support a host-mounted media directory, ensure the proxy image is built, and improve the “success” signal by checking multiple containers.
Changes:
- Bind-mount
media/from the host filesystem intowebandproxyindocker-compose.prod.local.yaml(replacing the namedmedia_volume). - Extend the install script to create a
media/directory and build the proxy image. - Update install-time verification to check
web,db, andproxycontainers (but currently with a shell syntax error and brittle timing).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| scripts/Linux/install-docker-env.sh | Creates a host media directory, builds the proxy, and verifies multiple containers after docker compose up. |
| docker/docker-compose.prod.local.yaml | Switches media from a named volume to a host bind mount and sets a fixed proxy container name. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| echo "Verifying that the containers are running..." | ||
| if [ "$(docker container inspect -f '{{.State.Status}}' "tekdb_web" 2>/dev/null)" = "running" ]; then | ||
| if [ "$(docker container inspect -f '{{.State.Status}}' "tekdb_web" 2>/dev/null)" = "running" ] && [ "$(docker container inspect -f '{{.State.Status}}' "db" 2>/dev/null)" = "running" ] && [ "$(docker container inspect -f '{{.State.Status}}' "tekdb_proxy" 2>/dev/null)" = "running" ]; then; | ||
| echo "TEKDB containers are running successfully." | ||
| else | ||
| echo "Failed to start TEKDB containers. Please check the Docker logs for more information." |
There was a problem hiding this comment.
I agree w/ Copilot here - that extra ; after the then breaks the syntax.
A: I don't think this runs (I can't run other local scripts with this syntax)
B: If it did run, I think the clause would be ended, the 'true' result would always run, and the 'false' result would never run (if bash didn't choke on interpreting else as a standalone commaned)
rhodges
left a comment
rhodges
left a comment
There was a problem hiding this comment.
Please fix syntax in install-docker-env.sh line 83.
Also the mkdir -p suggestions are worth considering.
| echo "Verifying that the containers are running..." | ||
| if [ "$(docker container inspect -f '{{.State.Status}}' "tekdb_web" 2>/dev/null)" = "running" ]; then | ||
| if [ "$(docker container inspect -f '{{.State.Status}}' "tekdb_web" 2>/dev/null)" = "running" ] && [ "$(docker container inspect -f '{{.State.Status}}' "db" 2>/dev/null)" = "running" ] && [ "$(docker container inspect -f '{{.State.Status}}' "tekdb_proxy" 2>/dev/null)" = "running" ]; then; | ||
| echo "TEKDB containers are running successfully." | ||
| else | ||
| echo "Failed to start TEKDB containers. Please check the Docker logs for more information." |
There was a problem hiding this comment.
I agree w/ Copilot here - that extra ; after the then breaks the syntax.
A: I don't think this runs (I can't run other local scripts with this syntax)
B: If it did run, I think the clause would be ended, the 'true' result would always run, and the 'false' result would never run (if bash didn't choke on interpreting else as a standalone commaned)
changes:
docker-compose.prod.local.yaml(task)