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

Avoid bumping the class serial when invoking executor #26684

Merged
merged 1 commit into from Oct 4, 2016

Conversation

Projects
None yet
5 participants
@matthewd
Member

matthewd commented Oct 2, 2016

Fixes #25068.

The general conclusion here is that we should avoid using the block form of set_callback within the framework. Users can choose the convenience and brevity, but (unless/until @tenderlove can prevent the serial increment on instance_eval) it has a trade-off we shouldn't be making for them.

@matthewd matthewd force-pushed the matthewd:executor-serial branch Oct 2, 2016

@matthewd matthewd force-pushed the matthewd:executor-serial branch 2 times, most recently Oct 2, 2016

@matthewd matthewd force-pushed the matthewd:executor-serial branch to e8b36e7 Oct 3, 2016

@kaspth

kaspth approved these changes Oct 3, 2016

@matthewd matthewd merged commit 4d6feef into rails:master Oct 4, 2016

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
codeclimate Code Climate didn't find any new or fixed issues.
Details

matthewd added a commit that referenced this pull request Oct 4, 2016

Merge pull request #26684 from matthewd/executor-serial
Avoid bumping the class serial when invoking executor
@matthewd

This comment has been minimized.

Member

matthewd commented Oct 4, 2016

@Paxa

This comment has been minimized.

Contributor

Paxa commented on activerecord/lib/active_record/query_cache.rb in e8b36e7 Oct 26, 2016

I think ActiveRecord::Base.connection will give connection associated with current thread or borrow one if no connection for current thread. So after it's called, ActiveRecord::Base.connected? will always be true. is it?

This comment has been minimized.

Member

matthewd replied Oct 26, 2016

Yeah, that condition was introduced in 160cc33, as part of a plan to better separate these parts some time later. That hasn't happened yet, but it seems fine to keep for now.

@matthewd matthewd deleted the matthewd:executor-serial branch Jun 11, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment