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

Support for period ranges #81

Closed
wants to merge 1 commit into from

Conversation

flevour
Copy link

@flevour flevour commented Aug 8, 2014

Supports range based periods, for the cases when periods needs to be manually defined such as start/end of the month or any particularly different metric.

period_based_on_proc = proc {|req| (Time.now.gmtime.beginning_of_month..Time.now.gmtime.end_of_month) }
Rack::Attack.throttle('req/ip', :limit => 100, :period => period_based_on_proc) do |req|
  req.ip
end

@zmillman
Copy link
Contributor

zmillman commented Aug 8, 2014

I'm skeptical about how useful this would be to 95% of rack-attack users would find helpful. (Is this feature worth the extra complexity?)

Do you have a specific use-case in mind?

@flevour
Copy link
Author

flevour commented Aug 11, 2014

@zmillman in my case I 'm using rack-attack to throttle access to an API.
The API plans allow a given amount of monthly requests where a month is a defined as a calendar month starting at midnight of 1st day and ending at midnight of last day.
Time.now.to_i/1.month doesn't give the same precision in terms of reset date.

On a second thought, instead of a range, I could change the logic to support an :until => Time option that could be used in place of :period.
WDYT?

@ktheory
Copy link
Collaborator

ktheory commented Aug 31, 2014

Thanks for this PR, @flevour. 😺

I agree with @zmillman that this is not generally useful as currently implemented with ranges.

The simplest change to support your use case is to make period take a proc. Then you can do period: -> { (Time.now.end_of_month - Time.now).to_i }.

This is similar to how the limit option works.

Gonna close this for now. Feel free to update to make period take a proc.

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

Successfully merging this pull request may close these issues.

3 participants