Deployment Fixes - #91
Conversation
…lery, itsdangerous, and monitoring/logging tools (structlog, sentry-sdk, flower)
There was a problem hiding this comment.
Pull request overview
This pull request adds deployment automation capabilities and enhances Python dependencies to support advanced features like job queuing, monitoring, and database migrations. The update script automates the process of updating Docker-based deployments, while the dependency changes add support for Celery job processing, structured logging, error monitoring, and Flask-Migrate for database migrations.
Changes:
- Added automated deployment update script (
deploy/update.sh) for Docker environments with support for both development and production configurations - Enhanced Python dependencies with job queue support (Celery, Redis), monitoring tools (Sentry, Flower, structlog), email capabilities (Flask-Mailman), and database migration management (Flask-Migrate)
- Updated
.gitignoreto exclude the job storage directory used by the Celery job queue system
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| deploy/update.sh | New automated update script for Docker deployments that pulls code, rebuilds containers, runs migrations, and performs health checks |
| requirements-ngc.txt | Added dependencies for environment management, migrations, job queuing, email, and monitoring; removed direct alembic dependency in favor of Flask-Migrate |
| .gitignore | Added exclusion for /webapp/job_storage/ directory used by Celery for job file storage |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Step 6: Restart Celery workers (if using job queue) | ||
| if docker ps --format '{{.Names}}' | grep -q "writebot-celery"; then | ||
| log_info "Restarting Celery workers..." | ||
| docker compose -f ${COMPOSE_FILE} restart celery-worker celery-beat 2>/dev/null || true |
There was a problem hiding this comment.
The script doesn't account for Docker Compose profiles used in production. The docker-compose.production.yml file uses profiles: [celery] for celery-worker and celery-beat services, which means these services won't be started by default with docker compose up -d. To restart Celery services in production when using profiles, you need to specify the profile explicitly, e.g., docker compose -f ${COMPOSE_FILE} --profile celery restart celery-worker celery-beat. Without this, the restart command on line 68 will fail for production deployments that use Celery.
| Flask-Limiter>=3.3.0 | ||
| Flask-Minify>=0.42 | ||
| flask-assets>=2.0 | ||
| Flask-Migrate>=4.0.0 |
There was a problem hiding this comment.
Removing the direct alembic dependency while the codebase still directly imports and uses it could cause issues. Files like webapp/init_db.py and webapp/manage_migrations.py import directly from alembic (e.g., from alembic.config import Config, from alembic import command). While Flask-Migrate includes alembic as a transitive dependency, relying on transitive dependencies is fragile - if Flask-Migrate changes its alembic dependency or pins a different version, the direct imports in your code could break. Consider keeping the explicit alembic>=1.13.0 dependency, or refactor the code to use Flask-Migrate's API instead of calling alembic directly.
| Flask-Migrate>=4.0.0 | |
| Flask-Migrate>=4.0.0 | |
| alembic>=1.13.0 |
| log_info "Waiting for health check..." | ||
| sleep 10 | ||
|
|
||
| if docker exec ${CONTAINER} python -c "import requests; r = requests.get('http://localhost:5000/api/health', timeout=5); exit(0 if r.status_code == 200 else 1)" 2>/dev/null; then |
There was a problem hiding this comment.
The health check script uses the requests library, but requests is not listed as a dependency in requirements-ngc.txt. The health check command on line 59 runs import requests; r = requests.get(...), which will fail if the requests library is not installed in the container. Either add requests to the requirements file or modify the health check to use a different approach, such as using curl or Python's built-in urllib.
| if docker exec ${CONTAINER} python -c "import requests; r = requests.get('http://localhost:5000/api/health', timeout=5); exit(0 if r.status_code == 200 else 1)" 2>/dev/null; then | |
| if docker exec ${CONTAINER} python -c "import urllib.request, sys; req = urllib.request.Request('http://localhost:5000/api/health'); r = urllib.request.urlopen(req, timeout=5); sys.exit(0 if r.getcode() == 200 else 1)" 2>/dev/null; then |
| # Step 1: Pull latest code (if using git) | ||
| if [ -d ".git" ]; then | ||
| log_info "Pulling latest code..." | ||
| git pull origin main |
There was a problem hiding this comment.
The git pull command hardcodes the branch name as "main", but some repositories may use "master" or other branch names. Consider using git pull without specifying the branch, which will pull the current branch's tracking remote, or make the branch name configurable through an environment variable or command-line argument.
| git pull origin main | |
| if [ -n "${UPDATE_BRANCH:-}" ]; then | |
| git pull origin "${UPDATE_BRANCH}" | |
| else | |
| git pull | |
| fi |
| log_error() { echo -e "${RED}[ERROR]${NC} $1"; } | ||
|
|
||
| # Determine compose file and container name | ||
| if [ "$1" == "--production" ]; then |
There was a problem hiding this comment.
Use single equals sign for string comparison in bash instead of double equals for better POSIX compliance and portability. While == works in bash, = is the standard POSIX comparison operator and is more portable across different shell implementations.
| if [ "$1" == "--production" ]; then | |
| if [ "$1" = "--production" ]; then |
| # Step 3: Run database migrations | ||
| log_info "Running database migrations..." | ||
| docker exec ${CONTAINER} flask db upgrade | ||
|
|
||
| # Step 4: Restart services with new code | ||
| log_info "Restarting services..." | ||
| docker compose -f ${COMPOSE_FILE} up -d | ||
|
|
There was a problem hiding this comment.
The migration command runs on a container that may still be using the old code. The sequence should be: (1) rebuild containers, (2) restart services with new code, (3) run migrations on the updated container. Currently, migrations run at step 3 (line 49) but the services are restarted at step 4 (line 53), which means migrations could run against old code if the container hasn't been recreated yet. Consider moving the migration step after the service restart.
| # Step 3: Run database migrations | |
| log_info "Running database migrations..." | |
| docker exec ${CONTAINER} flask db upgrade | |
| # Step 4: Restart services with new code | |
| log_info "Restarting services..." | |
| docker compose -f ${COMPOSE_FILE} up -d | |
| # Step 3: Restart services with new code | |
| log_info "Restarting services..." | |
| docker compose -f ${COMPOSE_FILE} up -d | |
| # Step 4: Run database migrations | |
| log_info "Running database migrations..." | |
| docker exec ${CONTAINER} flask db upgrade |
| fi | ||
|
|
||
| # Step 6: Restart Celery workers (if using job queue) | ||
| if docker ps --format '{{.Names}}' | grep -q "writebot-celery"; then |
There was a problem hiding this comment.
The grep pattern "writebot-celery" will not match the actual Celery container names. In the development docker-compose.yml, the containers are named "writebot-celery-worker" and "writebot-celery-beat", not "writebot-celery". The pattern should be changed to match the actual container names, for example using "writebot-celery-worker" or a pattern like "writebot-celery-.*" to match both worker and beat containers.
| if docker ps --format '{{.Names}}' | grep -q "writebot-celery"; then | |
| if docker ps --format '{{.Names}}' | grep -q "writebot-celery-"; then |
This pull request introduces an automated update script for Docker deployments and enhances the project's Python dependencies to improve environment management, database migrations, job queueing, and monitoring. The update script simplifies the process of updating and maintaining the application in both development and production environments, while the dependency updates add support for environment variables, database migrations, job queues, and monitoring tools.
Deployment Automation:
deploy/update.shscript that automates pulling the latest code, rebuilding Docker containers, running database migrations, restarting services, performing health checks, and restarting Celery workers if present. The script supports both development and production configurations and provides helpful output and troubleshooting tips.Dependency Enhancements:
python-dotenvanditsdangeroustorequirements-ngc.txtto improve environment variable management and security.Flask-Migratefor managing database migrations and removed the directalembicdependency, streamlining migration management.Flask-Mailmanfor email support andcelery[redis]for background job processing.structlogfor structured logging,sentry-sdkfor error monitoring, andflowerfor Celery monitoring.