feat: add lock name to SQL to trace the source#25
feat: add lock name to SQL to trace the source#25dkubb wants to merge 4 commits intoBetterment:mainfrom
Conversation
dkubb
force-pushed
the
dkubb/main/add/lock-name-to-sql
branch
2 times, most recently
from
f2ba5df to
c9c149a
Compare
|
question: @samandmoore not sure what the process is for PRs in this repo: Should I also bump the |
|
good question. i think yes. it's easiest to bump the version and update the changelog here and then i can merge it and do a release. i can do this |
bumping a patch version since this is a backward compatible tiny change
| Widget.with_transactional_lock('Widget.first') do | ||
| Widget.first.update!(name: 'once') | ||
|
|
||
| assert_pg_lock_comment do |
There was a problem hiding this comment.
is there a reason you're preferring to wrap this around every test instead of making a single separate test case for this assertion?
There was a problem hiding this comment.
comment: That's a good question. The tests weren't structured in a way that made the preferred layout obvious to me. They are very special cased to exercise the uncontended and contended contexts. I couldn't see any familiar patterns in the code for which one way would be better than another, so I opted to run it all the time.
In retrospect I still think it's something you'd want to run in the contended and uncontended contexts because the side-effects (i.e. the addition of the comment) needs to be asserted for both cases. There's not really a good way to hook into both of the contexts. Maybe I could've done it with an around block so it doesn't need to be explicit but I'm not sure if that would be better or not.
|
closing in to pull in after bigger change on main |
2 participants
Summary
Description
This branch adds the lock name to the SQL to allow the original lock name to be traced.