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

Memoize errors #20

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

Conversation

njonsson
Copy link

I don’t know whether you’d consider this to be within the scope of Memoist, but I found a need for it.

As it stands, an error raised by a method prevents its memoization. This commit allows Memoist to cache not just successful results of a method but also errors. If a method is called with the same arguments, the same Exception object is raised without an additional call to the method.

@pboling
Copy link
Contributor

pboling commented Jun 16, 2014

That's pretty cool. Not taking a stance on viability either way, but damn. That's cool.

@iainbeeston
Copy link

I would find this very useful

@iainbeeston
Copy link

For example, where the memoised function is expensive (eg network access, file IO or an expensive calculation)

@pboling
Copy link
Contributor

pboling commented Mar 26, 2015

👍 Bump. I really think this makes sense, after thinking about it for almost a year... ⌚ If the goal of memoizable is to cache the result of a method call, this fixes a bug where it hasn't been doing that for exceptions.

@njonsson Based on this comment I think this may require a fork and a new project with expanded scope.

@matthewrudy
Copy link
Owner

Thanks for your contribution.
And apologies I never looked at this before.

But...

Do you really think that's expected behaviour?

I guess it depends what you're trying to memoize.
But if it's something that can fail, I'd like to believe you want to retry it somehow.

Eg.

class User < ActiveRecord::Base
  has_many :posts

  def post_titles
    posts.pluck(:title)
  end
  memoize :post_titles
end

If this fails, then the database connection is dead.

If the database connection is dead, then what do we do?

Presumably we have a retry scheme. Or maybe that happens at a lower level
(eg deadlock retry)

So, I guess what I'm saying is "what's an example of an exception that we want to cache?"

I can only think of exceptions that we don't want to cache.

Now errors do not prevent memoization. If the method is called with the
same arguments, the same Exception object is raised without an
additional call to the method.
@njonsson
Copy link
Author

njonsson commented Apr 9, 2015

But if it’s something that can fail, I’d like to believe you want to retry it somehow.

The API you established for memoized values applies to memoized errors, too. I’ve updated the MiniTest test to demonstrate that specifying the optional :reload argument calls the underlying method again.

@cheerfulstoic
Copy link

What about making it a configuration option?

  • Give a boolean saying that, for this particular memoization, you would like to cache errors

and/or

  • Give it an array which contains errors which you would like to memoize

@sebjacobs sebjacobs force-pushed the master branch 2 times, most recently from 9d66c95 to 34f90dd Compare November 8, 2019 21:05
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.

5 participants