Skip to content

Refactor initialization script transaction management (PP-3684)#3048

Merged
jonathangreen merged 4 commits intomainfrom
chore/initialization-transaction-management
Feb 13, 2026
Merged

Refactor initialization script transaction management (PP-3684)#3048
jonathangreen merged 4 commits intomainfrom
chore/initialization-transaction-management

Conversation

@jonathangreen
Copy link
Member

@jonathangreen jonathangreen commented Feb 13, 2026

Note: This PR is stacked on #3047. Review only the changes from that branch.

Description

Refactor the initialization script's transaction management so that each sub-operation (schema creation, data initialization, Alembic migrations) manages its own transaction boundaries, rather than wrapping everything in a single long-lived connection and transaction.

Key changes:

  • pg_advisory_lock now accepts Engine | Connection and yields the lock Connection, removing the lock_id=None sentinel in favor of callers using nullcontext() directly.
  • InstanceInitializationScript.run() passes the Engine to pg_advisory_lock instead of creating a connection + transaction up front. The advisory lock is held on a dedicated connection while sub-operations use their own connections.
  • initialize_database_schema now scopes each step (schema DDL, data init, Alembic stamp) in its own transaction.
  • Extracted _db_initialized static method for checking whether the database has been initialized.
  • Added args parameter to run() to allow tests to pass arguments directly instead of relying on sys.argv.

Motivation and Context

This lays the groundwork for PP-3684, which requires a more well-defined transaction model for initialization.

How Has This Been Tested?

  • Updated unit tests in tests/manager/scripts/test_initialization.py
  • Updated advisory lock tests in tests/manager/sqlalchemy/test_util.py with parametrized Engine/Connection paths
  • Updated migration integration tests in tests/migration/test_instance_init_script.py

Checklist

  • I have updated the documentation accordingly.
  • All new and existing tests passed.

@jonathangreen jonathangreen force-pushed the chore/rename-test-id-fixture branch from 774bdfe to 4c5f7b1 Compare February 13, 2026 01:28
@jonathangreen jonathangreen force-pushed the chore/initialization-transaction-management branch from 2b922e9 to e26953d Compare February 13, 2026 01:29
@jonathangreen jonathangreen requested a review from a team February 13, 2026 01:32
@codecov
Copy link

codecov bot commented Feb 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.04%. Comparing base (bbe338c) to head (abe0ad7).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3048   +/-   ##
=======================================
  Coverage   93.04%   93.04%           
=======================================
  Files         480      480           
  Lines       43712    43716    +4     
  Branches     6028     6027    -1     
=======================================
+ Hits        40670    40677    +7     
+ Misses       1972     1968    -4     
- Partials     1070     1071    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Base automatically changed from chore/rename-test-id-fixture to main February 13, 2026 14:46
Copy link
Contributor

@tdilauro tdilauro left a comment

Choose a reason for hiding this comment

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

This is great! 🎉 🚀

Move from a single long-lived connection wrapping all initialization
to letting each sub-operation manage its own transaction boundaries.
The advisory lock is now held on a dedicated connection while schema
creation, data initialization, and Alembic migrations each use their
own connections and transactions.

Update pg_advisory_lock to accept Engine | Connection and yield the
lock connection, removing the lock_id=None sentinel in favor of
callers using nullcontext() directly.
@jonathangreen jonathangreen force-pushed the chore/initialization-transaction-management branch from e26953d to 2a532d7 Compare February 13, 2026 15:13
@jonathangreen jonathangreen enabled auto-merge (squash) February 13, 2026 15:26
@jonathangreen jonathangreen merged commit 83cf185 into main Feb 13, 2026
19 checks passed
@jonathangreen jonathangreen deleted the chore/initialization-transaction-management branch February 13, 2026 15:34
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.

2 participants