Skip to content

Group mount work by connection pool and use pool.with_connection#54

Closed
ngan wants to merge 1 commit into
np-fixturekit-insert-labelfrom
np-pool-with-connection
Closed

Group mount work by connection pool and use pool.with_connection#54
ngan wants to merge 1 commit into
np-fixturekit-insert-labelfrom
np-pool-with-connection

Conversation

@ngan
Copy link
Copy Markdown
Collaborator

@ngan ngan commented May 6, 2026

Summary

Mirror Rails' fixture loading idiom in ActiveRecordCoder#mount:

  • Group models by connection_pool instead of connection
  • Wrap the batch execution in pool.with_connection
  • Thread the explicit connection through build_delete_sql so we don't keep redundantly calling model.connection

Why

connection_pool is the stable identity for "this model's connection source." model.connection returns whatever connection the current thread/role happens to have checked out. pool.with_connection is the canonical lifecycle primitive — yields the leased connection if one exists, else checks out + checks in.

Behaviorally identical inside an rspec-rails transactional fixture (the test thread already has a connection leased), but more correct under role switching, multi-DB, or programmatic use of mount outside a test transaction.

Stack

Stacked on #53. Will rebase as that lands.

Test plan

  • Existing unit tests pass (the integration test "aggregates restore queries..." uses real pools that yield the real connection through with_connection)
  • CI green

🤖 Generated with Claude Code

Mirror Rails' fixture loading idiom: group models by connection_pool
(stable identity, role-aware) instead of connection, and wrap the
batch execution in pool.with_connection.

Behaviorally identical inside an rspec-rails transactional fixture
(with_connection yields the already-leased connection), but more
correct under role switching, multi-DB, or programmatic use of mount
outside a test transaction. Also stops calling model.connection
multiple times per mount.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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