-
Notifications
You must be signed in to change notification settings - Fork 116
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
Deprecate in favor of rack-attack #51
Conversation
@FreekingDean I don't really have an opinion; dependency updates would be due to the RDF.rb 3.1 release and dropping support for older Ruby versions, and of course versions of Rack < 2.0. I think the way to probably handle it is to do necessary updates to make this gem current, and add a [DEPRECATION] warning that points people to RackAttack and that this gem is now deprecated and will not receive any substantial updates. [DEPRECATION] probably needs to go in the gemspec post_install_message and all of the As the author and only person who can update RubyGems, @artob would need to either add an owner to the library (preferable) or do the push to RubyGems for whatever comes out of this. I see it's used by 236 different repos, although the gem spec looks pretty old. See https://github.com/ruby-rdf/rack-linkeddata/blob/develop/rack-linkeddata.gemspec for something that would be compatible with where we are with Ruby RDF. |
I have rubygem push access on this! I just didn't want to make a decision
in a vacuum :)
Thanks!
…--
Dean Galvin
Software Developer
M: 973-262-2132
On Wed, Dec 18, 2019 at 3:31 PM Gregg Kellogg ***@***.***> wrote:
@FreekingDean <https://github.com/FreekingDean> I don't really have an
opinion; dependency updates would be due to the RDF.rb 3.1 release and
dropping support for older Ruby versions, and of course versions of Rack <
2.0.
I think the way to probably handle it is to do necessary updates to make
this gem current, and add a [DEPRECATION] warning that points people to
RackAttack and that this gem is now deprecated and will not receive any
substantial updates. [DEPRECATION] probably needs to go in the gemspec
post_install_message and all of the initialize methods for each module.
As the author and only person who can update RubyGems, @artob
<https://github.com/artob> would need to either add an owner to the
library (preferable) or do the push to RubyGems for whatever comes out of
this.
I see it's used by 236 different repos, although the gem spec looks pretty
old.
See
https://github.com/ruby-rdf/rack-linkeddata/blob/develop/rack-linkeddata.gemspec
for something that would be compatible with where we are with Ruby RDF.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#51?email_source=notifications&email_token=AAOAJXMB67Q6SHZWRIV7WXLQZKCATA5CNFSM4I4XAMWKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHHMX4Q#issuecomment-567200754>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAOAJXONZUMETK72V67OPV3QZKCATANCNFSM4I4XAMWA>
.
|
627a46b
to
e17926c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a very sound adjustment. 👍🏻 on the merge
Just one data point, but I found this quickly, but didn't notice rack-attack until quite a while later.
Thanks that must be new :)
Thanks!
…--
Dean Galvin
Software Developer
M: 973-262-2132
On Fri, Sep 25, 2020 at 10:13 AM sandstrom ***@***.***> wrote:
***@***.**** commented on this pull request.
I think this is a very sound adjustment. 👍🏻 on the merge
Just one data point, but I found this quickly, but didn't notice
rack-attack until quite a while later.
------------------------------
In README.md
<#51 (comment)>:
> @@ -1,3 +1,13 @@
+DEPRECATED We suggest using rack-attack instead
+===============================================
+
+<https://github.com/kickstarter/rack-attack> Accomplishes the same goal as rack-throttle,
⬇️ Suggested change
-<https://github.com/kickstarter/rack-attack> Accomplishes the same goal as rack-throttle,
+<https://github.com/rack/rack-attack> Accomplishes the same goal as rack-throttle,
------------------------------
In lib/rack/throttle/limiter.rb
<#51 (comment)>:
> @@ -23,6 +23,7 @@ class Limiter
# @option options [String] :message ("Rate Limit Exceeded")
# @option options [String] :type ("text/plain; charset=utf-8")
def initialize(app, options = {})
+ warn "[DEPRECATION] `rack-throttle` is deprecated. Please use consider using `rack-attack` https://github.com/kickstarter/rack-attack instead."
⬇️ Suggested change
- warn "[DEPRECATION] `rack-throttle` is deprecated. Please use consider using `rack-attack` https://github.com/kickstarter/rack-attack instead."
+ warn "[DEPRECATION] `rack-throttle` is deprecated. Please use consider using `rack-attack` https://github.com/rack/rack-attack instead."
------------------------------
In rack-throttle.gemspec
<#51 (comment)>:
> @@ -38,5 +38,9 @@ Gem::Specification.new do |gem|
gem.add_runtime_dependency 'rack', '>= 1.0.0'
- gem.post_install_message = nil
+ gem.post_install_message = <<-POST
+rack-throttle is no longer under active development. Please consider
+using https://github.com/kickstarter/rack-attack instead as it is
⬇️ Suggested change
-using https://github.com/kickstarter/rack-attack instead as it is
+using https://github.com/rack/rack-attack instead as it is
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#51 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAOAJXODAEUSQTV4DIMG443SHSQQPANCNFSM4I4XAMWA>
.
|
@dryruby @FreekingDean any movement on this? This would help save some time for developers looking into rate limiting for rack-based applications. |
Co-authored-by: sandstrom <mail+github@a16m.se>
This is a really large conceptual PR so I'm going to let this sit for a while to gather feedback!
RackAttack has much more active maintenance & is more mature. I believe we should nudge those looking to do rate limiting in rack apps that way to be the most helpful we can be!
Fixes #43
Fixes #55