Skip to content

[RateLimiter] add enabled param to rate_limiter config #14655

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

Closed
wants to merge 1 commit into from

Conversation

hosseinsalemi
Copy link

No description provided.

@OskarStark OskarStark added this to the 5.2 milestone Dec 5, 2020
@carsonbot carsonbot changed the title add enabled param to rate_limiter config [RateLimiter] add enabled param to rate_limiter config Dec 5, 2020
@OskarStark OskarStark requested a review from wouterj December 5, 2020 11:44
@wouterj
Copy link
Member

wouterj commented Dec 5, 2020

Hi @hosseinsalemi! If I'm correct, you should not need this option as it's enabled whenever you're configuring on of the rate limiters below it. Did you have any other experience with this?

@pierredup
Copy link
Contributor

@wouterj I think the issue here is, without that config, you get an Undefined index: enabled error.

There is a PR at symfony/symfony#39442 to fix this behaviour to catch cases when enabled is not defined

@wouterj
Copy link
Member

wouterj commented Dec 11, 2020

Alright, as the fix on symfony/symfony will be merged, I'm going to close the PR here (this addition becomes redundant after 5.2.1, which will probably be released in at most 2 weeks time).

Thank you @pierredup for providing all these fixes to the rate limiter docs & code!

@wouterj wouterj closed this Dec 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants