Skip to content
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

undefined method `maximum=' for String #8

Open
christoph-buente opened this issue Oct 31, 2014 · 4 comments
Open

undefined method `maximum=' for String #8

christoph-buente opened this issue Oct 31, 2014 · 4 comments

Comments

@christoph-buente
Copy link

Hey,

I'm trying to throttle the usage of my api only. This is the config i use:

# config.ru
uri = URI.parse(AppConfig.redis_url || 'http://localhost:6379')
require 'redis'
require 'rack/throttle'

use Rack::Throttle::SlidingWindow, rules: { url: /\/api/ },
                               average: 2,
                               burst:  10,
                               cache: Redis.new( host: uri.host, port: uri.port, password: uri.password ),
                               key_prefix: :throttle

And this is, what is happenIng:

Unexpected error while processing request: undefined method `maximum=' for "#<Rack::Throttle::SlidingWindow::LeakyBucket:0x007fb2938796f8>":String
~/project/vendor/bundle/ruby/2.1.0/gems/improved-rack-throttle-0.8.0/lib/rack/throttle/limiters/sliding_window.rb:31:in `allowed?'
~/project/vendor/bundle/ruby/2.1.0/gems/improved-rack-throttle-0.8.0/lib/rack/throttle/limiters/limiter.rb:39:in `call'

I suspect the marshalling/unmarshalling of the LeakyBucket object does not work correctly with redis.

@christoph-buente
Copy link
Author

BTW, it works fine with a memcached client.

@bensomers
Copy link
Owner

I think you're exactly right - looks like I've been relying on memcached doing automatic serialization, and Redis just saves a string representation rather than a proper serialized object. I'll take some time over the weekend and tweak the SlidingWindow Limiter to save more gracefully for Redis.

I think the other strategies should all work fine - the obvious workarounds until I can cut a release are to use a different strategy or a different cache store (as you're obv. already doing).

@bensomers
Copy link
Owner

Ok, fix is ready on branch redis-compat (0e9d398) - just added Marshaling of LeakyBuckets before caching them. I did not have the opportunity yet to set up a Redis test instance to play with - I'd like to before cutting a release. Would appreciate if you get a chance to test it on your app, as that would save me the trouble.

@christoph-buente
Copy link
Author

I tried to use another strategy before with redis. Not sure anymore, if it was successfull. But when ich changed the strategy it would reuse the same cache key. So it would try to call maximum= on a String, which was actually an ip adress or a time stamp. Not sure anymore. I think it would be a smart idea to change the keys depending on the strategy, so they do not interfere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants