This project is archived and is in readonly mode.

#2969 ✓wontfix
CodeMonkeyKevin

[Patch] Added Global expires_in for cache_store

Reported by CodeMonkeyKevin | July 28th, 2009 @ 11:55 PM

Patch allows mem_cache_store to have a default(global) expires_in parameter.

e.g
config.cache_store = :mem_cache_store, 'localhost:11211', {:expires_in => 1800}

From: Kevin Patel kevin@edge14.com and Joshua Borton digitaltoad@gmail.com

p.s. Moved expires_in method from cache.rb to mem_cache_store.rb as mem_cache is the only store that calls it.

Comments and changes to this ticket

  • CancelProfileIsBroken

    CancelProfileIsBroken September 25th, 2009 @ 01:01 PM

    • Tag changed from cache, mem_cache to bugmash, cache, mem_cache
  • Elad Meidar

    Elad Meidar September 25th, 2009 @ 03:56 PM

    +1 on idea, patch applies cleanly on 2-3-stable, tests pass (cause there aren't any :) ). Patch fails on master, i applied a patch too.
    I can't believe this was missing up until now, seems like a pretty mandatory fix.

    I can't seem to find a proper way to test it, can't find any recollection of expires_in tests in neither *_cache_test.rb test... ideas?

  • jroes

    jroes September 26th, 2009 @ 09:20 PM

    I've attached a patch. The original patch moves the expires_in method to MemCacheStore. I preserved the original location in case another store needs to be able to default this. I also included a test.

    Notice This is my first Rails patch. Please give me all the tips and suggestions you can.

  • Ben Marini

    Ben Marini September 26th, 2009 @ 11:27 PM

    -1 on implementation. After some thought, I don't think this fits with the current cache setting api, or belongs in activesupport.

    Currently, the api is:
    config.cache_store = [symbol_for_cache_class], [extra args passed to constructor]

    For example:
    ActionController::Base.cache_store = :mem_cache_store, "localhost"

    Thing is, MemCache does not take a default expires_in option in it's constructor. This functionality doesn't exist in the memcache-client gem. I think a better way to implement this would be a subclass of MemCache that accepts that option:
    config.cache_store = MemCacheWithDefaultExpires.new "localhost", :expires_in => 10.minutes

    Or submit this patch to the memcache-client gem. It's not Rails's responsibility to add extra functionality on top of caching implementations.

  • John Guenin

    John Guenin September 26th, 2009 @ 11:59 PM

    -1 I agree with Ben. This functionality would make a great patch for the memcache-client gem, but does not belong in ActiveSupport.

  • CancelProfileIsBroken

    CancelProfileIsBroken September 27th, 2009 @ 11:45 AM

    • Tag changed from bugmash, cache, mem_cache to cache, mem_cache
    • State changed from “new” to “wontfix”

    Agreed. Good work, but this functionality belongs upstream from us.

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

People watching this ticket

Pages