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

Allowing a "no cache" option instead of default DictCache #44

Closed
Kyria opened this issue Sep 8, 2016 · 2 comments
Closed

Allowing a "no cache" option instead of default DictCache #44

Kyria opened this issue Sep 8, 2016 · 2 comments
Milestone

Comments

@Kyria
Copy link
Contributor

Kyria commented Sep 8, 2016

At the moment, if we want to use pycrest without caching, we have 2 options :

Moreover, when you do EVE(cache=None) you may expect that pycrest do not cache anything instead of using a default "DictCache" that you may not want and that will make your process size in RAM grow a lot...

Why :
Because in some use, you don't want to cache data for multiple reasons.

Simple case: getting all EVE Online orders, in all regions (through region.marketOrderAll). if you take just the 64 k-space regions, you will save in the process memory / memcached / other, all the data.
In the case you do it in a batch, you don't really care if it's cached or not, but you care about CPU perf and RAM, and the latter is the issue here.
If you loop through every regions, with a DictCache, you'll end with a python process size from 1.5Go to 2Go, with 90% of this size used by the cache (in my case, the process grew to 2.4Go with it, but only ~250Mo with the above lines commented)
I could have used a specific cache system (memcached, redis, or anything else), but i will just move ~1.5-2Go (useless) data from my python process to another process, so it's not the right solution.

Possible Solution(s) :

  • if cache=None then set no cache / DummyCache instead of using a "hidden default DictCache"
  • or change the get method to something like def get(self, resource, params={}, caching=True):, to be able to disable manually the cache on some requests
@Kyria Kyria changed the title Allowing a "no cache" option, and no default to "dict cache" Allowing a "no cache" option instead of default DictCache Sep 8, 2016
@hkraal
Copy link
Contributor

hkraal commented Sep 8, 2016

I totally can find myself in the suggested solutions (None should indeed imply absence of any cache and DictCache should be default). Being able to disabled cache per request (taking CREST it's wierd caching timers into account) would certainly add value as well.

@Kyria
Copy link
Contributor Author

Kyria commented Oct 28, 2016

I close this, as the PR #47 has been merged

@Kyria Kyria closed this as completed Oct 28, 2016
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

No branches or pull requests

2 participants