Skip to content

Handle cache failures - #16

Open
mogman1 wants to merge 8 commits into
mxenabled:masterfrom
mogman1:handle_redis_failure
Open

Handle cache failures#16
mogman1 wants to merge 8 commits into
mxenabled:masterfrom
mogman1:handle_redis_failure

Conversation

@mogman1

@mogman1mogman1 commented Apr 22, 2023•
edited
Loading

Copy link
Copy Markdown

Fixes warnings and specs, adds handling for upstream cache failure by falling back to RPC call. Also sets up github workflow to automatically run pipelines for multiple ruby / activesupport versions.

@mogman1
mogman1force-pushed the handle_redis_failure branch 4 times, most recently from 8801dfd to 8ad3b32CompareApril 23, 2023 04:54
skunkworker
skunkworker reviewed Apr 25, 2023
View reviewed changes
Comment threadlib/active_remote/cached.rb Outdated
@mogman1
mogman1force-pushed the handle_redis_failure branch from 8ad3b32 to f239104CompareApril 28, 2023 03:33
@mogman1mogman1 changed the title Handle redis failureHandle cache failuresApr 28, 2023
@mogman1
mogman1force-pushed the handle_redis_failure branch 4 times, most recently from 78ab837 to f33f858CompareApril 28, 2023 05:22
@mogman1
mogman1force-pushed the handle_redis_failure branch from f33f858 to ff8f75dCompareApril 28, 2023 05:29
mogman1
mogman1 commented Apr 28, 2023
View reviewed changes
Comment threadspec/spec_helper.rb
end

def write(key, value)
def write(key, value, _options = nil)

@mogman1mogman1Apr 28, 2023

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Updated these to match signatures of Activesupport::Cache::Store

Comment threadspec/active_remote/cached/argument_keys_spec.rb
describe ::ActiveRemote::Cached::ArgumentKeys do
it "does not mutate a string by default" do
::ActiveRemote::Cached::ArgumentKeys.new("hello", {}).cache_key.must_equal("hello")
_(::ActiveRemote::Cached::ArgumentKeys.new("hello", {}).cache_key).must_equal("hello")

@mogman1mogman1Apr 28, 2023

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

These types of changes were made due to deprecation warnings from Minitest about calling methods like must_equal directly on objects.

skunkworker
skunkworker reviewed Apr 28, 2023
View reviewed changes
Comment thread.github/workflows/ci.yml
skunkworker
skunkworker reviewed Apr 28, 2023
View reviewed changes
Comment threadlib/active_remote/cached.rb Outdated
@mogman1

mogman1 commented Apr 28, 2023

Copy link
Copy Markdown
Author

As a note, I'd definitely prefer to publish a release candidate version of this first to do extra validation. Things worked as I expected testing locally, but abundance of caution and all that.

film42
film42 reviewed May 1, 2023
View reviewed changes
Comment threadlib/active_remote/cached.rb Outdated
film42
film42 reviewed May 1, 2023
View reviewed changes
Comment threadlib/active_remote/cached/cache.rb Outdated
@film42

film42 commented May 1, 2023

Copy link
Copy Markdown

LGTM

film42
film42 approved these changes May 3, 2023
View reviewed changes

@film42film42 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚀

@mogman1
mogman1force-pushed the handle_redis_failure branch from 28d02f1 to ca09ddfCompareMay 6, 2023 23:37
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@skunkworkerskunkworkerskunkworker left review comments
+1 more reviewer
@film42film42film42 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mogman1@film42@skunkworker