Skip to content

proof of concept for setting version number env variable #218

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
paigewilliams wants to merge 1 commit into
base: paige-docker-testing
Choose a base branch
from
Draft

proof of concept for setting version number env variable #218

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions .github/workflows/build-docker-image.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
name: Build Docker Image on Release

on:
push
Copy link

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing colon after push. The on trigger syntax requires a colon. This should be:

on:
  push:

or to trigger on all pushes:

on: push

The current syntax is invalid and will cause the workflow to fail.

Suggested change
push
push:

Copilot uses AI. Check for mistakes.

# on:
# release:
# types: [published]

jobs:
build:
runs-on: ubuntu-latest
Copy link
Member

@rhodges rhodges Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we run on:

  • ubuntu-latest
  • ubuntu 24.04 LTS
  • ubuntu 22.04 LTS

We may also have partners on Ubuntu 20.04 LTS, which is now EOL, but I don't think we'll be dockerizing them - instead we'll migrate.


steps:
- name: Checkout
uses: actions/checkout@v4

- name: Set version
run: echo "APP_VERSION=${GITHUB_REF_NAME#v}" >> .env
Copy link

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Writing APP_VERSION to a .env file won't make it available to docker-compose. The .env file created here is in the GitHub Actions runner's working directory, but docker-compose looks for .env in the build context.

Instead, you should either:

  1. Export it as an environment variable: echo "APP_VERSION=${GITHUB_REF_NAME#v}" >> $GITHUB_ENV
  2. Or pass it directly when running docker-compose (as shown in the commented line 22)

Additionally, when the workflow is triggered by push events (as currently configured), GITHUB_REF_NAME will be the branch name, not a version tag. This will only work correctly when triggered by release events.

Suggested change
run: echo "APP_VERSION=${GITHUB_REF_NAME#v}" >> .env
run: echo "APP_VERSION=${GITHUB_REF_NAME#v}" >> $GITHUB_ENV

Copilot uses AI. Check for mistakes.

# - name: Build images
# run: docker compose build --build-arg APP_VERSION=${GITHUB_REF_NAME#v}
2 changes: 1 addition & 1 deletion TEKDB/TEKDB/settings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@
TINYMCE_FILEBROWSER = False

# Add Version to the admin site header
VERSION = "2.2.2"
VERSION = os.getenv("APP_VERSION", "unknown")
Copy link
Member

@rhodges rhodges Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like if this is launched any way other than with Docker (looking at this variable in build-docker-image.yml and referenced again in docker-compose.yml) then the admin header will read 'unknown'. I don't think this is appropriate. I appreciate the goal of only setting this once: it's DRY and prevents conflict from potential future mis-matches, but the canonical source for this value must be platform agnostic -- at least until all deployments adopt a Docker-based system.
For now I recommend either replacing "unknown" with something meaningful like "2.2.2+" or "2.3+" or "2.3.x" or even "2.x" until the appropriate single source that works for all deployments is solved for.

ADMIN_SITE_HEADER = os.environ.get(
"ADMIN_SITE_HEADER", default="ITK DB Admin v{}".format(VERSION)
)
Expand Down
3 changes: 3 additions & 0 deletions TEKDB/docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ services:
build:
context: .
dockerfile: Dockerfile
args:
APP_VERSION: ${APP_VERSION}
Comment on lines +24 to +25
Copy link

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Dockerfile needs to accept the APP_VERSION build argument and set it as an environment variable. Add these lines near the top of the Dockerfile (after the base image):

ARG APP_VERSION=unknown
ENV APP_VERSION=${APP_VERSION}

Without these lines, the build arg passed from docker-compose.yml won't be captured, and the environment variable won't be available at runtime.

Copilot uses AI. Check for mistakes.
restart: unless-stopped
depends_on:
- db
Expand All @@ -35,6 +37,7 @@ services:
SQL_PASSWORD: postgrespw
SECRET_KEY: 'change-me'
DEBUG: '0'
APP_VERSION: ${APP_VERSION}
ports:
- "8000:8000"
volumes:
Expand Down
Loading