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

[ci skip] [docs] make increment requirement explicit #50909

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jamiemccarthy
Copy link
Contributor

Motivation / Background

As of December, ActionController::Metal::RateLimiting provides rate limiting through the increment method of any cache store. RateLimiting relies on a feature of the underlying store. This feature needs to be documented, so that it remains supported, and so alternative stores know to support it.

This PR has been created to document this behavior.

Detail

The feature is that increment, when passed expires_in, will set expiration time when the key is first created, and ignore that option if the key already exists.

That functionality is already tested here, in the CacheIncrementDecrementBehavior test module.

This Pull Request changes the documentation of ActiveSupport::Cache::Store.increment to reflect a now-required feature of existing stores.

@jamiemccarthy
Copy link
Contributor Author

Cc #50781

@jamiemccarthy
Copy link
Contributor Author

The force-push from a88748f to 30a914d was just a rebase, no changes in the branch.

@zzak
Copy link
Member

zzak commented May 18, 2024

@jamiemccarthy I think we're intentionally saying "Options are passed to the underlying cache implementation." to avoid duplicating effort to document all possible options here.

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.

2 participants