n8 / multi_fetch_fragments

Multi-fetch Fragments makes rendering and caching a collection of template partials easier and faster.
http://ninjasandrobots.com/rails-faster-partial-rendering-and-caching
MIT License
539 stars 63 forks source link

Dalli 2.6 requires mutable keys for read_multi and write #7

Closed bwillis closed 11 years ago

bwillis commented 11 years ago

I was playing around and doing some performance measurements, but came across this error in my test app:

dalli (2.6.0) lib/active_support/cache/dalli_store.rb:270:in `force_encoding'
dalli (2.6.0) lib/active_support/cache/dalli_store.rb:270:in `expanded_key'
dalli (2.6.0) lib/active_support/cache/dalli_store.rb:128:in `block in read_multi'
dalli (2.6.0) lib/active_support/cache/dalli_store.rb:128:in `each'
dalli (2.6.0) lib/active_support/cache/dalli_store.rb:128:in `inject'
dalli (2.6.0) lib/active_support/cache/dalli_store.rb:128:in `read_multi'
multi_fetch_fragments (0.0.9) lib/multi_fetch_fragments.rb:32:in `render_collection_with_multi_fetch_cache'

It seems that the 2.6.0 bump brought in https://github.com/mperham/dalli/commit/aa7ff874dc3e522ad9255f363705a660d6727e7a which now requires that the given keys to cache.read_multi and cache.write must be mutable.

soma commented 11 years ago

Oh I just added this fix as well.. :)

n8 commented 11 years ago

Thanks guys for both looking at this and the code. @soma, I went with this pull because it only duped the keys in one place in the code. But I'd like to get your other test merged in if you want to create another pull request with it?

soma commented 11 years ago

There :)

On Tue, Dec 18, 2012 at 4:01 AM, Nathan Kontny notifications@github.comwrote:

Thanks guys for both looking at this and the code. @somahttps://github.com/soma, I went with this pull because it only duped the keys in one place in the code. But I'd like to get your other test merged in if you want to create another pull request with it?

— Reply to this email directly or view it on GitHubhttps://github.com/n8/multi_fetch_fragments/pull/7#issuecomment-11472132.