Docker Demo #2

lukakoning · 2025-12-15T10:54:35Z

Makes Docker demo run fully in Docker + add automated test to verify Docker demo works

…d of host script)

This reverts commit 4f63408.

Copilot

Pull request overview

This PR eliminates the dependency on host-side bash scripts by enabling the Docker demo to run entirely within Docker containers using native Docker Compose commands. It also introduces automated CI testing via GitHub Actions to verify the demo pipeline works correctly.

Key changes:

Replaced custom run_demo.sh wrapper with direct Docker Compose commands
Added GitHub Actions workflow to automatically test the Docker demo in CI
Enhanced docker-compose.yml with profiles for different use cases (db-only, external database)

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 11 comments.

File	Description
docker/demo/docker-compose.yml	Refactored service names, added db-only profile, updated entrypoints to handle line endings and permissions at runtime, improved documentation
docker/demo/demo_entrypoint.sh	Changed shebang to `#!/usr/bin/env bash` and modified error handling to support minimal environments
docker/demo/README.md	Updated all examples to use direct Docker Compose commands instead of wrapper script
.github/workflows/docker-demo.yaml	Added new CI workflow to build, run, and validate the Docker demo with comprehensive error handling

Copilot · 2025-12-15T11:00:03Z

docker/demo/docker-compose.yml

        condition: service_healthy
    working_dir: /app
-    entrypoint: ["/bin/bash", "/app/docker/demo/demo_entrypoint.sh"]
+    entrypoint: ["/bin/bash", "-lc", "sed -i 's/\r$//' /app/docker/demo/demo_entrypoint.sh && chmod +x /app/docker/demo/demo_entrypoint.sh && /app/docker/demo/demo_entrypoint.sh"]


Using bash -lc in the entrypoint is unusual and potentially problematic. The -l flag makes bash run as a login shell, which sources profile files and can introduce unexpected behavior or side effects. Unless login shell behavior is specifically required, use bash -c instead for more predictable execution.

Suggested change

entrypoint: ["/bin/bash", "-lc", "sed -i 's/\r$//' /app/docker/demo/demo_entrypoint.sh && chmod +x /app/docker/demo/demo_entrypoint.sh && /app/docker/demo/demo_entrypoint.sh"]

entrypoint: ["/bin/bash", "-c", "sed -i 's/\r$//' /app/docker/demo/demo_entrypoint.sh && chmod +x /app/docker/demo/demo_entrypoint.sh && /app/docker/demo/demo_entrypoint.sh"]

Copilot · 2025-12-15T11:00:04Z

.github/workflows/docker-demo.yaml

+          docker exec ggmpilot-demo-db bash -lc "\
+            set -euo pipefail;\
+            psql -U postgres -d demo -v ON_ERROR_STOP=1 -c 'SELECT count(*) AS client_rows FROM silver.client;' \
+              || psql -U postgres -d demo -v ON_ERROR_STOP=1 -c 'SELECT count(*) AS client_rows FROM silver."CLIENT";';\
+          "


The validation step uses bash -lc to run commands inside the container, which is inconsistent with the use of bash (without -l) elsewhere in the workflow. The login shell flag (-l) is unnecessary here and can introduce unexpected behavior from profile files. Use bash -c for consistency and more predictable execution.

Copilot · 2025-12-15T11:00:04Z

docker/demo/demo_entrypoint.sh

-set -euo pipefail
+#!/usr/bin/env bash
+
+set -euo errexit


The syntax set -euo errexit is incorrect because -e is already the short form of errexit. This line should be set -eu (since pipefail is set separately on line 7 with fallback). The current form mixes the short flag -e with the long option name errexit, which is redundant and may cause unexpected behavior.

Suggested change

set -euo errexit

set -eu

Copilot · 2025-12-15T11:00:05Z

docker/demo/docker-compose.yml

        condition: service_healthy
    working_dir: /app
-    entrypoint: ["/bin/bash", "/app/docker/demo/demo_entrypoint.sh"]
+    entrypoint: ["/bin/bash", "-lc", "sed -i 's/\r$//' /app/docker/demo/demo_entrypoint.sh && chmod +x /app/docker/demo/demo_entrypoint.sh && /app/docker/demo/demo_entrypoint.sh"]


The entrypoint uses sed to remove carriage returns and chmod to make the script executable. However, this approach is fragile because it processes the file at runtime on every container start. A better approach would be to ensure the file has correct line endings in the repository (via .gitattributes) and set execute permissions in the Dockerfile. This runtime workaround suggests the root cause (Windows line endings) should be fixed at the source.

Suggested change

entrypoint: ["/bin/bash", "-lc", "sed -i 's/\r$//' /app/docker/demo/demo_entrypoint.sh && chmod +x /app/docker/demo/demo_entrypoint.sh && /app/docker/demo/demo_entrypoint.sh"]

entrypoint: ["/app/docker/demo/demo_entrypoint.sh"]

Copilot · 2025-12-15T11:00:05Z

.github/workflows/docker-demo.yaml

+        run: |
+          docker pull postgres:16
+          docker pull python:3.12-slim
+          docker pull ghcr.io/astral-sh/uv:latest || true


The workflow uses docker pull ghcr.io/astral-sh/uv:latest || true which silently ignores pull failures. While this might be intentional for cache warmup, it could mask network issues or repository access problems. Consider logging when the pull fails to make troubleshooting easier.

Suggested change

docker pull ghcr.io/astral-sh/uv:latest || true

docker pull ghcr.io/astral-sh/uv:latest || echo "WARNING: Failed to pull ghcr.io/astral-sh/uv:latest (non-fatal)"

Copilot · 2025-12-15T11:00:05Z

docker/demo/docker-compose.yml

+    image: ${DEMO_APP_IMAGE:-ggmpilot:demo}
    build:
      context: ../..
      dockerfile: Dockerfile


The image is specified after the build section, but Docker Compose best practice is to specify image before build. While this works, the conventional order is to define the image name first, then the build context. This makes it clearer what image will be created.

Copilot · 2025-12-15T11:00:06Z

docker/demo/docker-compose.yml

      - external
    working_dir: /app
-    entrypoint: ["/bin/bash", "/app/docker/demo/demo_entrypoint.sh"]
+    entrypoint: ["/bin/bash", "-lc", "sed -i 's/\r$//' /app/docker/demo/demo_entrypoint.sh && chmod +x /app/docker/demo/demo_entrypoint.sh && /app/docker/demo/demo_entrypoint.sh"]


Using bash -lc in the entrypoint is unusual and potentially problematic. The -l flag makes bash run as a login shell, which sources profile files and can introduce unexpected behavior or side effects. Unless login shell behavior is specifically required, use bash -c instead for more predictable execution.

Suggested change

entrypoint: ["/bin/bash", "-lc", "sed -i 's/\r$//' /app/docker/demo/demo_entrypoint.sh && chmod +x /app/docker/demo/demo_entrypoint.sh && /app/docker/demo/demo_entrypoint.sh"]

entrypoint: ["/bin/bash", "-c", "sed -i 's/\r$//' /app/docker/demo/demo_entrypoint.sh && chmod +x /app/docker/demo/demo_entrypoint.sh && /app/docker/demo/demo_entrypoint.sh"]

Copilot · 2025-12-15T11:00:06Z

docker/demo/docker-compose.yml

      - external
    working_dir: /app
-    entrypoint: ["/bin/bash", "/app/docker/demo/demo_entrypoint.sh"]
+    entrypoint: ["/bin/bash", "-lc", "sed -i 's/\r$//' /app/docker/demo/demo_entrypoint.sh && chmod +x /app/docker/demo/demo_entrypoint.sh && /app/docker/demo/demo_entrypoint.sh"]


The entrypoint uses sed to remove carriage returns and chmod to make the script executable. However, this approach is fragile because it processes the file at runtime on every container start. A better approach would be to ensure the file has correct line endings in the repository (via .gitattributes) and set execute permissions in the Dockerfile. This runtime workaround suggests the root cause (Windows line endings) should be fixed at the source.

Suggested change

entrypoint: ["/bin/bash", "-lc", "sed -i 's/\r$//' /app/docker/demo/demo_entrypoint.sh && chmod +x /app/docker/demo/demo_entrypoint.sh && /app/docker/demo/demo_entrypoint.sh"]

entrypoint: ["/bin/bash", "-lc", "/app/docker/demo/demo_entrypoint.sh"]

Copilot · 2025-12-15T11:00:06Z

docker/demo/docker-compose.yml

+    image: ${DEMO_APP_IMAGE:-ggmpilot:demo}
    build:
      context: ../..
      dockerfile: Dockerfile


The image is specified after the build section, but Docker Compose best practice is to specify image before build. While this works, the conventional order is to define the image name first, then the build context. This makes it clearer what image will be created.

Copilot · 2025-12-15T11:00:06Z

.github/workflows/docker-demo.yaml

+          docker exec ggmpilot-demo-db psql -U postgres -d demo -v ON_ERROR_STOP=1 -c "\
+            SELECT table_schema, table_name\
+            FROM information_schema.tables\
+            WHERE table_schema IN ('source','staging','silver')\
+            ORDER BY 1,2;\


The bash command embeds SQL that spans multiple lines within the docker exec command. The backslashes and line continuation make this hard to read and maintain. Consider extracting the SQL query to a variable or using a here-document for better readability.

Suggested change

docker exec ggmpilot-demo-db psql -U postgres -d demo -v ON_ERROR_STOP=1 -c "\

SELECT table_schema, table_name\

FROM information_schema.tables\

WHERE table_schema IN ('source','staging','silver')\

ORDER BY 1,2;\

docker exec ggmpilot-demo-db bash -c "psql -U postgres -d demo -v ON_ERROR_STOP=1 <<'EOF'

SELECT table_schema, table_name

FROM information_schema.tables

WHERE table_schema IN ('source','staging','silver')

ORDER BY 1,2;

EOF

- Used shared test helpers more consistently - Centralized port assignment to avoid conflicts

- Used shared test helpers more consistently - Centralized port assignment to avoid conflicts - Improve retry behaviour in get_connection

lukakoning added 10 commits December 15, 2025 08:33

Rename (k2b -> K2B, cssd -> CSSD)

0ca2eff

Make Docker demo fully containerized (start via Docker Compose instea…

4a28695

…d of host script)

Add Docker demo test

0ac61f4

Update Docker demo commands

3c1ee38

Fix case in py import

d231824

Fix Docker demo test

58551c2

Rename (k2b -> K2B, cssd -> CSSD)

4f63408

Merge branch 'main' into feat/docker-demo

70bb3e0

Revert "Rename (k2b -> K2B, cssd -> CSSD)"

54b5b8f

This reverts commit 4f63408.

Merge branch 'main' into feat/docker-demo

64a1e3c

Copilot AI review requested due to automatic review settings December 15, 2025 10:54

Copilot started reviewing on behalf of lukakoning December 15, 2025 10:55 View session

Copilot AI reviewed Dec 15, 2025

View reviewed changes

lukakoning added 2 commits December 15, 2025 12:59

Improve test robustness

6b8dfe2

- Used shared test helpers more consistently - Centralized port assignment to avoid conflicts

Merge branch 'main' into feat/docker-demo

fcf42f0

lukakoning force-pushed the main branch from 6b8dfe2 to edf2448 Compare December 15, 2025 13:21

lukakoning added 2 commits December 15, 2025 14:21

Improve test robustness

edf2448

- Used shared test helpers more consistently - Centralized port assignment to avoid conflicts - Improve retry behaviour in get_connection

Merge branch 'main' into feat/docker-demo

38d3f23

lukakoning force-pushed the main branch from edf2448 to cb4fe14 Compare December 15, 2025 13:40

lukakoning added 3 commits December 15, 2025 14:40

Improve test robustness

cb4fe14

- Used shared test helpers more consistently - Centralized port assignment to avoid conflicts - Improve retry behaviour in get_connection

Merge branch 'main' into feat/docker-demo

0b7dafb

Improve test robustness

7b19e49

- Used shared test helpers more consistently - Centralized port assignment to avoid conflicts - Improve retry behaviour in get_connection

lukakoning force-pushed the main branch from cb4fe14 to 7b19e49 Compare December 15, 2025 16:50

Merge branch 'main' into feat/docker-demo

43d7c50

lukakoning force-pushed the main branch 2 times, most recently from f0630f6 to 4fd500a Compare December 15, 2025 21:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Docker Demo #2

Docker Demo #2

Uh oh!

lukakoning commented Dec 15, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 15, 2025

Uh oh!

Copilot AI Dec 15, 2025

Uh oh!

Copilot AI Dec 15, 2025

Uh oh!

Copilot AI Dec 15, 2025

Uh oh!

Copilot AI Dec 15, 2025

Uh oh!

Copilot AI Dec 15, 2025

Uh oh!

Copilot AI Dec 15, 2025

Uh oh!

Copilot AI Dec 15, 2025

Uh oh!

Copilot AI Dec 15, 2025

Uh oh!

Copilot AI Dec 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	entrypoint: ["/bin/bash", "-lc", "sed -i 's/\r$//' /app/docker/demo/demo_entrypoint.sh && chmod +x /app/docker/demo/demo_entrypoint.sh && /app/docker/demo/demo_entrypoint.sh"]
	entrypoint: ["/bin/bash", "-c", "sed -i 's/\r$//' /app/docker/demo/demo_entrypoint.sh && chmod +x /app/docker/demo/demo_entrypoint.sh && /app/docker/demo/demo_entrypoint.sh"]

	entrypoint: ["/bin/bash", "-lc", "sed -i 's/\r$//' /app/docker/demo/demo_entrypoint.sh && chmod +x /app/docker/demo/demo_entrypoint.sh && /app/docker/demo/demo_entrypoint.sh"]
	entrypoint: ["/app/docker/demo/demo_entrypoint.sh"]

	docker pull ghcr.io/astral-sh/uv:latest \|\| true
	docker pull ghcr.io/astral-sh/uv:latest \|\| echo "WARNING: Failed to pull ghcr.io/astral-sh/uv:latest (non-fatal)"

	entrypoint: ["/bin/bash", "-lc", "sed -i 's/\r$//' /app/docker/demo/demo_entrypoint.sh && chmod +x /app/docker/demo/demo_entrypoint.sh && /app/docker/demo/demo_entrypoint.sh"]
	entrypoint: ["/bin/bash", "-lc", "/app/docker/demo/demo_entrypoint.sh"]

Docker Demo #2

Are you sure you want to change the base?

Docker Demo #2

Uh oh!

Conversation

lukakoning commented Dec 15, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants