Merge default_options, and add default_options_overwrite - #26
Merged
Merged
Conversation
Each call to default_options replaced the options. In a Rails app, the
railtie sets :expires_in and :race_condition_ttl, and then the app
initializers run. An initializer that called default_options with other
options removed the TTL. Every cached finder call with no :expires_in then
wrote an entry that never expired.
abacus shows the problem. Its initializer calls
default_options(:active_remote_cached_replace_characters => true), and 34 of
its 43 cached finder call sites wrote Redis keys with no TTL. In a January
2024 prod key dump, 1,641 of 1,944 of those keys had no expiry.
default_options now merges the given options into the current options, so
a later call adds to them. The new default_options_overwrite replaces the
options, for a caller that wants the old behavior or wants to clear them.
default_options({}) no longer clears the options. The specs used it to
reset state between examples, so they now call default_options_overwrite({}).
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
default_options now merges the given options into the current options,
and default_options({}) no longer clears them. Use
default_options_overwrite({}) to clear them.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Each call to
ActiveRemote::Cached.default_optionsreplaced the options. In a Rails app, the railtie sets:expires_in(5 minutes) and:race_condition_ttl(5 seconds), and then the app initializers run. An initializer that calleddefault_optionswith other options removed the TTL. Every cached finder call with no:expires_inthen wrote an entry that never expired.The README said "In Rails apps, the :race_condition_ttl option defaults to 5 seconds". That was false for any app that called
default_options.Changes
default_options(hash)merges the hash into the current options. A key in the new hash wins. With no argument ornil, it reads the options, as before.default_options_overwrite(hash)replaces the options and returns them.default_options_overwrite({})clears them.default_options[:key] = valuestill works.default_options_overwrite({}).VERSIONis1.3.0.Breaking change
default_options({})no longer clears the options. It now does nothing. Usedefault_options_overwrite({}).Apps that call
default_optionswith other options get the railtie TTL back, so their finders call the remote service more often.This PR bumps the version to 1.3.0, and the README has an "Upgrading to 1.3.0" section.
Evidence
7 new specs. 6 fail on
master, and all pass with this change.Full suite: 189 examples, 0 failures (Ruby 3.4.9). RuboCop: 18 files, no offenses.
A Rails 7.1 app (JRuby 10.0.6.0) whose initializer calls
default_options(:active_remote_cached_replace_characters => true):🤖 Generated with Claude Code