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

Cache functionality? #29

Open
pbaranay opened this issue Nov 4, 2018 · 5 comments
Open

Cache functionality? #29

pbaranay opened this issue Nov 4, 2018 · 5 comments
Labels
enhancement New feature or request

Comments

@pbaranay
Copy link

pbaranay commented Nov 4, 2018

Would you accept a pull request that adds caching functionality to Scrython? Supporting caching in the library itself would be a big benefit, rather than each other developer needing to implement their own caching scheme.

If you're excited by this, I'm willing to look into adding this feature, and I'd love to discuss implementation details in more depth. My main thoughts are currently the following:

  • I'd like to use diskcache for this, specifically the FanoutCache.memoize decorator. Given how many objects Scryfall has, I'd rather use a disk-based cache (with a long expiration time) than a memory-based one.
  • The caching functionality should probably be disabled by default.

Thanks for writing the library!

@NandaScott
Copy link
Owner

I'd have to take a look at this in greater detail later, but I'd be okay with seeing this implemented.

Since a major refactor on Scrython was finished, there's a new Foundation class that was added that all other classes inherit from. New functionality like that would be easiest to implement in that class. Make a pull to develop and I'd be happy to take a look at it.

@NandaScott NandaScott added the enhancement New feature or request label Nov 6, 2018
@jkaupanger
Copy link

Did anything ever come of this?

@ghost
Copy link

ghost commented Nov 24, 2020

Is this still planned because I would like to use it?

@NandaScott
Copy link
Owner

I currently don't have the time to dedicate to updating this package with that kind of functionality. I also never planned to add this, it was suggested by someone else. I only have time for small patches. I welcome anyone who would like to fork this repo and submit a PR to add it.

@Investigamer
Copy link

I might see about giving this a shot, will submit a PR if/when I'm successful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants