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

[Enhancement]: Improve pending migration(s) check #2013

Conversation

aybruhm
Copy link
Member

@aybruhm aybruhm commented Aug 22, 2024

Description

This PR refactors the migration handling process to improve efficiency and accuracy. Key changes include:

Changes Included

  • Updated the get_pending_migration_head function to compare only the latest migration head with the current state.
  • Enhanced the get_current_migration_head_from_db function to ensure it returns a single migration head, improving consistency.

What to QA

  • Migration Check (For First-time User):
    1. Delete all Docker resources related to Agenta on your PC.

    2. Run docker compose up --build to create new images, volumes, and containers.

    3. Apply the Alembic migration with this command:

      docker exec -w /app/agenta_backend/migrations/postgres agenta-backend-1 alembic -c alembic.oss.ini upgrade head
  • Migration Check (For Returning User):
    1. Run docker compose up --build to recreate the containers.

    2. Apply the Alembic migration with this command:

      docker exec -w /app/agenta_backend/migrations/postgres agenta-backend-1 alembic -c alembic.oss.ini upgrade head

Acceptance Tests

  • Verify that after applying the migrations, you do not receive a notification about pending migrations in the terminal.
  • Ensure to test for both OSS and cloud-dev environments.

NOTE: Cloud (prod) and OSS (prod) do not require QA as notifying users about pending migrations is a local concern.

…atest migration head

- Refactored the migration check to compare the current database migration head against the latest migration script head.
- Enhanced `get_current_migration_head_from_db` to ensure only one migration head is tracked.
- Updated logic in `get_pending_migration_head` to reduce unnecessary checks and handle first-time setup more efficiently.
- Improved error handling and clarified logging messages for better debugging.
Copy link

vercel bot commented Aug 22, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
agenta ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 22, 2024 3:58pm
agenta-documentation ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 22, 2024 3:58pm

@aybruhm aybruhm changed the base branch from main to feature/age-540-endpoint-for-app-type August 22, 2024 15:55
@aybruhm aybruhm marked this pull request as ready for review August 22, 2024 16:25
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. Backend enhancement New feature or request labels Aug 22, 2024
@aybruhm aybruhm changed the title Enhancement/refactor migration handling [Enhancement]: Improve pending migration check Aug 22, 2024
@aybruhm aybruhm changed the title [Enhancement]: Improve pending migration check [Enhancement]: Improve pending migration(s) check Aug 22, 2024
@aybruhm aybruhm requested a review from jp-agenta August 22, 2024 16:26
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Aug 27, 2024
@aybruhm
Copy link
Member Author

aybruhm commented Aug 29, 2024

@aybruhm aybruhm merged commit 8736de4 into feature/age-540-endpoint-for-app-type Aug 29, 2024
10 checks passed
@aybruhm aybruhm deleted the enhancement/refactor-migration-handling branch August 29, 2024 08:27
@zenUnicorn
Copy link
Contributor

All tests run successfully for both new and returning users ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backend enhancement New feature or request lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants