-
Notifications
You must be signed in to change notification settings - Fork 151
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
Add new adapter for ActiveRecord 4.2, fix #198 #200
Conversation
efbceec
to
adfc880
Compare
@@ -11,7 +11,7 @@ group :development do | |||
gem 'activerecord', "= #{ENV['activerecord'] || '4.1.8'}" | |||
gem 'em-mongo' | |||
gem 'bson_ext' | |||
gem 'mysql2' |
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.
Any reason why you're freezing the version?
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.
I'm not sure why, but it failed to load an adapter https://travis-ci.org/igrigorik/em-synchrony/builds/102289375:
Specified 'em_mysql2' for database adapter, but the gem is not loaded. Add `gem 'mysql2'` to your Gemfile.
Probably, it is related to the issue rails/rails#21544 (found it here). This fix rails/rails@5da5e37 hasn't been released yet.
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.
Opened a separate PR #203 for it
@igrigorik hello! why you don't like an original MonitorMixin approach? |
Related: * NullTransaction rails/rails@057c237 * TransactionManager rails/rails@97bb76d
adfc880
to
7a9f802
Compare
Add new adapter for ActiveRecord 4.2, fix #198
\o/ ... thank you for working on this! |
Yet another try to add support for ActiveRecord 4.2 and fix #198.
It just adds a new adapter by replacing
ClosedTransaction
withNullTransaction
and usingActiveRecord::ConnectionAdapters::TransactionManager
. Generally it keeps the same behavior.There is also another PR #190 which changes
MonitorMixin
to add fiber aware support.