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

Exceptions on the cache-store should be ignored for Read/WriteThroughStore #225

Merged
merged 2 commits into from
Apr 4, 2014

Conversation

solar
Copy link
Contributor

@solar solar commented Apr 3, 2014

I'm not sure that those behaviors are intended or not.
But using Future#respond variants like .onFailure { case x: Exception => storeValue } are meaningless.
(onFailure won't transform into new Future, it will simply register a callback)

Use Future#transform variants instead of Future#responds on XxxThroughStore

…Store

Use Future#transform variants instead of respond's on XxxThroughStore
@@ -0,0 +1,13 @@
package com.twitter.storehaus
Copy link
Contributor

Choose a reason for hiding this comment

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

This file needs the copyright/license header

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, thanks

@rubanm
Copy link
Contributor

rubanm commented Apr 3, 2014

@solar Thanks for the fix. Just curious if you've tried this combinator in the wild?

configurable possibility of exceptions
@solar
Copy link
Contributor Author

solar commented Apr 4, 2014

I was trying to use it (not in production yet).
And when I read the code, noticed some differences between the documentation and the implementation.

rubanm added a commit that referenced this pull request Apr 4, 2014
Exceptions on the cache-store should be ignored for Read/WriteThroughStore
@rubanm rubanm merged commit e2334cf into twitter:develop Apr 4, 2014
@rubanm
Copy link
Contributor

rubanm commented Apr 4, 2014

@solar Great, I'm interested in learning more about how you're using this, with which backing stores etc. Merged. Thanks.

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