Ensure no permanent DB connections are used - #28
Conversation
| # correctly, thus the clock_timestamp() value would be frozen between calls. We don't want that here. | ||
| # See https://stackoverflow.com/questions/73184531/why-would-postgres-clock-timestamp-freeze-inside-a-rails-unit-test | ||
| upserted = @model_class.connection.uncached { @model_class.connection.select_one(sql) } | ||
| upserted = Pecorino.with_connection_or_fallback(@model_class) { |connection| connection.uncached { connection.select_one(sql) } } |
There was a problem hiding this comment.
I believe you can do @model_class.connection_pool.with_connection for all those cases instead. ..._or_fallback is not really needed as you will always have a pool with ActiveRecord these days
There was a problem hiding this comment.
@julik see https://github.com/cheddar-me/pecorino/actions/runs/16647409861/job/47111305387 the tests for rails 7 were failing because with_connection is not defined. I had initially built it using with_connection directly but it might be helpful to preserve backwards compatibility
There was a problem hiding this comment.
True - but that is on the ActiveRecord subclass, what I'm suggesting is using the pool instead - so @model_class.connection_pool.with_connection, not @model_class.with_connection
There was a problem hiding this comment.
Done but in the block tests AR did not have a pool so had to build one in the setup but no big deal
aed26e1 to
93a662c
Compare
624b17b to
fb915fd
Compare
svanhesteren
left a comment
svanhesteren
left a comment
There was a problem hiding this comment.
👍 Thanks and good to go. Let's see how this improves things.
3 participants
Replace
.connectionwith.connection_pool.with_connectionfor proper connection managementWhat
Refactored all direct
.connectioncalls to use.with_connectionblocks in adapters, tests, and documentation.Why