[FEATURE] Add Docker Setup and Documentation - #1023
Conversation
|
| } | ||
| raise SampleNotFoundException(f"Extra file {additional_id} for sample {sample.id} not found") | ||
| raise SampleNotFoundException(f"Sample with id {sample_id} not found") | ||
| raise SampleNotFoundException(f"Sample with id {sample_id} not found") No newline at end of file |
There was a problem hiding this comment.
Remove this change please.
| service-account.json | ||
| gcp-key.json | ||
| secret_key | ||
| secret_csrf No newline at end of file |
There was a problem hiding this comment.
Add a newline to the end.
| @@ -0,0 +1,64 @@ | |||
| services: | |||
| # --- 1. Database Service --- | |||
There was a problem hiding this comment.
Please remove all these unnecessary comments.
| python3 -c " | ||
| import json | ||
| from cryptography.hazmat.primitives.asymmetric import rsa | ||
| from cryptography.hazmat.primitives import serialization | ||
|
|
||
| try: | ||
| key = rsa.generate_private_key(public_exponent=65537, key_size=2048) | ||
| pem = key.private_bytes(serialization.Encoding.PEM, | ||
| serialization.PrivateFormat.TraditionalOpenSSL, | ||
| serialization.NoEncryption()).decode() | ||
| except Exception as e: | ||
| print(f'WARNING: Key generation failed: {e}') | ||
| pem = 'DUMMY_KEY' | ||
|
|
||
| sa = { | ||
| 'type': 'service_account', | ||
| 'project_id': 'docker-dev', | ||
| 'private_key_id': 'docker-dev-key', | ||
| 'private_key': pem, | ||
| 'client_email': 'docker-dev@docker-dev.iam.gserviceaccount.com', | ||
| 'client_id': '000000000000', | ||
| 'auth_uri': 'https://accounts.google.com/o/oauth2/auth', | ||
| 'token_uri': 'https://oauth2.googleapis.com/token', | ||
| } | ||
| with open('$REAL_SA_PATH', 'w') as f: | ||
| json.dump(sa, f, indent=2) | ||
| " |
There was a problem hiding this comment.
I'd rather see extra helper files (in the correct subfolder) than almost unreadable python in a shell script.
| @@ -0,0 +1,292 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
This file is way too large for a entrypoint of a docker. A lot of these things should be handled at build time (ensuring dirs exist/created etc).
| @@ -0,0 +1,81 @@ | |||
| FROM python:3.11-slim-bullseye | |||
There was a problem hiding this comment.
I'd rather see a more recent python version.
| # ============================================================ | ||
|
|
||
| # ---------- MySQL ---------- | ||
| MYSQL_ROOT_PASSWORD=root |
There was a problem hiding this comment.
The SP does not need to know the root password for MYSQL
| # Port exposed on the HOST for the Flask app (container always listens on 5000) | ||
| APP_PORT=5000 | ||
| # Port exposed on the HOST for direct MySQL access (optional, for debugging) | ||
| DB_EXTERNAL_PORT=3306 |
There was a problem hiding this comment.
This should not be exposed.
|
2 participants
[FEATURE]
In raising this pull request, I confirm the following (please check boxes):
My familiarity with the project is as follows:
Docker development environment
Two containers — MySQL 8 and the application under Gunicorn — so a contributor
can get a working platform without installing MySQL, Python and the native
libraries the app needs:
cp env.example .env # edit the passwords docker compose up --buildThat is the whole setup. The app comes up on http://localhost:5000 with a
schema, fixture data and a browsable UI.
Why the entrypoint doesn't just run
flask db upgradeIt can't — and this is the part worth reviewing.
Replaying the migration chain against an empty database fails:
Migration
2e0d2e02a721drops foreign keys by the names MySQL generatesautomatically. Those names only exist if the schema was first built from the
models, which is what
database.create_sessiondoes on the app's first run.So the chain assumes a database that was never created by the chain itself.
That is a pre-existing property of the repository, not something Docker
introduced, and it is why nothing here tries to "fix" the migrations.
install/init_schema.pytherefore branches on whetheralembic_versionexists:
stamp()it at head,which is what a normal first run does anyway
migrations that are newer
It deliberately never does both:
create_allwould create a table that apending migration still expects to create itself, which then fails.
A fresh database is also seeded with the existing
install/sample_db.py,because several pages dereference rows they assume are present — the home page
does
GeneralData.query.filter(...).first().valuewith no null check, so anempty schema alone serves a 500 on
/. Seeding through the repository's ownscript keeps that fix out of application code.
Files
Dockerfiledocker-compose.ymldocker-entrypoint.shconfig.docker.pyconfig.pyfor the container, all values from envinstall/wait_for_db.pyinstall/init_schema.pyinstall/generate_dev_credentials.pyenv.example,DOCKER.md,.dockerignoreOne change to application code
utility.py—serve_file_downloadunconditionally calledstorage_client_bucket.blob(...), which cannot work without a real bucket, soevery download raised in development.
It now serves the file from
SAMPLE_REPOSITORYwhen no bucket is configured.A configured bucket still takes precedence and production behaviour is
unchanged, with one exception worth naming: a
GoogleAPIErrorfrom agenuinely configured bucket now falls through to the local copy and returns
404 if it isn't there, where it previously raised. Happy to narrow that to
only the unconfigured case if you would rather production keep failing loudly.
Nothing else outside the Docker files is touched. The earlier
mod_sample/controllers.pyedit from review has been dropped.Security
uid 1001), not rootenv.exampleships placeholders, and secretkeys plus dev GCP credentials are generated at image build
MYSQL_ROOT_PASSWORDorMYSQL_PASSWORDareunset rather than defaulting to something guessable
unprivileged user