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

Allow disabling search throttling via config or envvar #1738

Merged
merged 1 commit into from
Mar 8, 2023

Conversation

thatbudakguy
Copy link
Member

@thatbudakguy thatbudakguy commented Mar 8, 2023

Ref #1572

Since this sets the default to disabling search throttling, we can test that it's working correctly and then file a PR in puppet to set THROTTLE_SEARCHES to true for prod only to re-enable it in that environment. After that I think the issue can be closed.

@thatbudakguy thatbudakguy marked this pull request as ready for review March 8, 2023 00:27
# don't throttle requests with a q, because it's more likely to be a real user
next if req.params['q'].present?
# throttle bot-like search queries if configured
if ActiveModel::Type::Boolean.new.cast(ENV.fetch("THROTTLE_SEARCHES", Settings.throttle_searches))
Copy link
Contributor

@jcoyne jcoyne Mar 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about using SETTINGS__THROTTLE_SEARCHES as the env var? Then we can just do Settings.throttle_searches. The config gem can cast to boolean and load from ENV vars if properly configured.

See https://github.com/sul-dlss/dlme/blob/main/config/initializers/config.rb#L15-L24

We'd also want to set env_parse_values = true https://github.com/sul-dlss/dlme/blob/main/config/initializers/config.rb#L34

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing using the console and this worked exactly how I'd expect...pretty cool. made the change

@thatbudakguy thatbudakguy force-pushed the configurable-throttle branch from df42103 to 7e90fe8 Compare March 8, 2023 18:35
@thatbudakguy thatbudakguy requested a review from jcoyne March 8, 2023 20:02
@jcoyne jcoyne merged commit ef1593c into main Mar 8, 2023
@jcoyne jcoyne deleted the configurable-throttle branch March 8, 2023 20:46
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.

2 participants