-
Notifications
You must be signed in to change notification settings - Fork 69
feat: support of multidb #116
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
Conversation
3832cbc
to
6732dda
Compare
BREAKING CHANGE: Remove private APIs (Base, DatabaseAdapterSupport). Add full mixed adapter support for PostgreSQL/MySQL in same app. Add JRuby compatibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR merges and simplifies CI configuration, removes legacy adapter support code, and refactors advisory‐lock implementation into modular concerns.
- Introduces new modules for core advisory logic, MySQL adapters, JRuby support, and lock stack items
- Removes legacy
Base
,DatabaseAdapterSupport
, old gemfiles, Appraisals, and separate workflow ymls - Updates Docker Compose, Rakefile, Makefile, Gemfile, and consolidates CI into a single
.github/workflows/ci.yml
Reviewed Changes
Copilot reviewed 55 out of 55 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
lib/with_advisory_lock/mysql_advisory.rb | New MySQL advisory‐lock concern |
lib/with_advisory_lock/core_advisory.rb | Core advisory logic and stack management |
lib/with_advisory_lock/jruby_adapter.rb | JRuby compatibility layer for injecting advisory modules |
lib/with_advisory_lock/concern.rb | Public API for advisory-lock methods on ActiveRecord::Base |
lib/with_advisory_lock/lock_stack_item.rb | Defines a simple data object for tracking locks |
lib/with_advisory_lock.rb | Entry point: autoloads, ActiveSupport hooks, debug logging, JRuby hook |
docker-compose.yml | Upgrades Postgres image and remaps host ports |
Rakefile | Adds frozen literal, rails tasks setup, and loads dummy app tasks |
Makefile | Simplifies test target, adds setup-db, removes per-adapter targets |
Gemfile | Adds default gems, versioned ActiveRecord, dotenv, railties |
.github/workflows/ci.yml | Single consolidated CI workflow for Ruby/Rails matrix |
Multiple old gemfiles, Appraisals, ci-postgresql.yml, ci-mysql8.yml | Removed legacy appraisal/gemfile and workflow configurations |
Comments suppressed due to low confidence (5)
lib/with_advisory_lock.rb:9
- The autoload references 'with_advisory_lock/result' but no corresponding
lib/with_advisory_lock/result.rb
was added. Add the missing file or remove this autoload to prevent load errors.
autoload :Result, 'with_advisory_lock/result'
lib/with_advisory_lock.rb:14
- The autoload references 'with_advisory_lock/postgresql_advisory' but no corresponding
lib/with_advisory_lock/postgresql_advisory.rb
was added. Add the missing file or remove this autoload to prevent load errors.
autoload :PostgreSQLAdvisory, 'with_advisory_lock/postgresql_advisory'
lib/with_advisory_lock.rb:17
- The autoload references 'with_advisory_lock/failed_to_acquire_lock' but no corresponding
lib/with_advisory_lock/failed_to_acquire_lock.rb
was added. Add the missing file or remove this autoload to prevent load errors.
autoload :FailedToAcquireLock, 'with_advisory_lock/failed_to_acquire_lock'
lib/with_advisory_lock.rb:5
- [nitpick] Consider requiring 'active_support/concern' explicitly (and any other needed parts of ActiveSupport) to ensure
ActiveSupport::Concern
and related extensions are loaded correctly.
require 'active_support'
lib/with_advisory_lock.rb:23
- [nitpick] Using
puts
for debug logging can clutter STDOUT; consider using a proper logger or removing this in production code.
puts '[WithAdvisoryLock] Loaded into ActiveRecord' if ENV['DEBUG_LOAD']
Co-authored-by: Copilot <[email protected]>
closes #85 #95 #100
The Problem
Previously, if you had 2 adapters of different types, you had to segregate your app or this gem would randomly patch stuff. Like, imagine having PostgreSQL for your main models and MySQL for analytics - the gem would just
pick one adapter type and YOLO apply it to everything. Your MySQL models would try to call
pg_advisory_lock
. Fun times.Since we're in 2025 and not the 1950s, I upgraded this gem to follow Rails' multi-DB architecture properly. I tried many time to do this without much success but lot of learning.
What Was Broken
The old architecture used a singleton
Base
class that would detect your database type on first use and remember it forever. So if your first model happened to be PostgreSQL, congrats - all your MySQL models now thinkthey're PostgreSQL too. They will use the wrong pronouns and dialect.
JRuby users? LOL, the gem just didn't work at all because JRuby loads adapters differently and our ActiveSupport hooks never fired.
The Fix
prepend
Breaking Changes
If you were using private APIs like
WithAdvisoryLock::Base
orDatabaseAdapterSupport
- sorry not sorry, they're gone. The public API stays exactly the same because I'm not a monster.JRuby Note
@headius @enebo - Quick question: Are you planning to add support for ActiveSupport's adapter-specific load hooks (
:active_record_postgresqladapter
,:active_record_mysql2adapter
)?Right now we have a special
jruby_adapter.rb
file that hooks into the connection method because these hooks don't fire on JRuby. If this is a special-needs architecture thing, we'll keep our workaround. If it's something you'replanning to support, we can eventually clean this up.
TL;DR
Multi-database setups now work correctly. Each adapter gets its own implementation. JRuby users can finally use this gem. Welcome to 2025.