Skip to content
This repository has been archived by the owner on May 3, 2023. It is now read-only.

Switch to Bose/cache from Jim-Lambert-Bose/cache, catch more instances of covering up underlying errors with cache miss #5

Merged
merged 3 commits into from
Aug 13, 2020

Conversation

Ryan-Rose-Bose
Copy link
Contributor

I was not thorough enough in my first PR 🙃

This further fixes instances where cache operation errors are being returned as ErrCacheMiss when they may be other underlying errors. This completes this fix for all entry types in Get, and also in Delete.

Copy link

@Peter-Tanski-Bose Peter-Tanski-Bose left a comment

Choose a reason for hiding this comment

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

This looks good. Clients may have to review their error handling.

@Ryan-Rose-Bose
Copy link
Contributor Author

@Peter-Tanski-Bose they may, but if anything it would be to improve error handling, as there aren't any interface changes!

@Ryan-Rose-Bose
Copy link
Contributor Author

For this to properly work (for Delete() specifically), there needs to be an update to Jim's cache library, which I've put up for review here: Bose/cache#12

@Ryan-Rose-Bose
Copy link
Contributor Author

Ryan-Rose-Bose commented Aug 5, 2020

Sorry for the spam, but while talking to @Alex-Grant-Bose and realizing no one has access to Jim-Lambert-Bose/cache, we decided to fork Jim-Lambert-Bose/cache into BoseCorp/cache. I will update this PR switching to that repo as a dependency once it's set up properly.

The new PR exists here: BoseCorp/cache#1

@Ryan-Rose-Bose Ryan-Rose-Bose changed the title Catch more instances of covering up underlying errors with cache miss Switch to BoseCorp/cache from Jim-Lambert-Bose/cache, catch more instances of covering up underlying errors with cache miss Aug 5, 2020
@Ryan-Rose-Bose Ryan-Rose-Bose changed the title Switch to BoseCorp/cache from Jim-Lambert-Bose/cache, catch more instances of covering up underlying errors with cache miss Switch to Bose/cache from Jim-Lambert-Bose/cache, catch more instances of covering up underlying errors with cache miss Aug 13, 2020
Copy link

@Peter-Tanski-Bose Peter-Tanski-Bose left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link

@Alex-Grant-Bose Alex-Grant-Bose left a comment

Choose a reason for hiding this comment

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

LGTM

@Ryan-Rose-Bose Ryan-Rose-Bose merged commit e2fc741 into master Aug 13, 2020
@Ryan-Rose-Bose Ryan-Rose-Bose deleted the rtr-cache-err2 branch August 13, 2020 13:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants