Skip to content
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.

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

Feat: improved migrations #426

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Conversation

nimish-ks
Copy link
Member

@nimish-ks nimish-ks commented Jan 3, 2025

🔍 Overview

This PR introduces a more robust Django database migration strategy for multi-container deployments by separating migrations into a dedicated job. This change addresses potential race conditions that can occur when multiple Django containers attempt to run migrations simultaneously during deployment. This also maintains the support for legacy (self-hosted) deployment migrations.

Docs: phasehq/docs#127

💡 Proposed Changes

  • Added a dedicated migrations service in docker-compose
  • Modified backend Dockerfile to respect external migration flag
  • Updated backend and worker services to depend on successful migration completion
  • Introduced environment variables to control migration behavior:
    • EXTERNAL_MIGRATION: Controls whether migrations are handled externally

📝 Release Notes

Added

  • New migrations service for safer database migrations in multi-container setups
  • Environment variable EXTERNAL_MIGRATION to control migration behavior

Changed

  • Backend and worker containers now wait for migrations to complete before starting
  • Updated dependency chain in docker-compose files
  • Migration logic separated from application startup

Migration Steps

  1. For single-instance deployments: No action required (migrations continue to work as before)
  2. For multi-instance deployments: Set EXTERNAL_MIGRATION=true in environment variables

🧪 Testing

  • Tested migration service startup order
  • Verified backend starts after successful migrations
  • Tested backward compatibility with single-instance deployments
  • Verified worker process doesn't attempt migrations

🎯 Reviewer Focus

  1. Migration service implementation in docker-compose files
  2. Migration flag logic in the backend Dockerfile
  3. Verify that the EXTERNAL_MIGRATION flags works as expected
  4. Service dependency chain and startup order
  5. Backward compatibility with existing deployments

➕ Additional Context

This change is based on Django deployment best practices for handling migrations in containerized environments. It prevents potential database corruption that could occur when multiple containers attempt migrations simultaneously.

✨ How to Test the Changes Locally

  1. Pull the latest changes
  2. Start the stack with docker-compose:
    docker-compose up -d
  3. Verify migration service completes successfully:
    docker-compose logs migrations
  4. Verify backend and worker start after migrations:
    docker-compose logs backend worker

💚 Did You...

  • Ensure linting passes (code style checks)?
  • Update dependencies and lockfiles (if required)
  • Regenerate graphql schema and types (if required)
  • Verify the app builds locally?
  • Manually test the changes on different browsers/devices?
  • Test migration service with both new and existing databases?
  • Verify backward compatibility with single-instance deployments?

@nimish-ks nimish-ks marked this pull request as draft January 4, 2025 09:09
@nimish-ks nimish-ks marked this pull request as ready for review January 4, 2025 10:56
@nimish-ks nimish-ks self-assigned this Jan 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant