pycrest / PyCrest

Python library for accessing the EVE Online CREST API
MIT License
35 stars 22 forks source link

Use non-volatile hash for caching keys #30

Closed hkraal closed 8 years ago

hkraal commented 8 years ago

Currently the "hash()" function is being used in the FileCache._getpath function. This function however doesn't return the same value (and thus path) between seperate python runs as it's using a salt.

This renders the cache function useless unless we made the same call multiple times within the same run but in that case the data would be returned from EVE()._data, not APIObject().get.

def _getpath(self, key):    # pragma: no cover
  return os.path.join(self.path, str(hash(key)) + '.cache')
wtfrank commented 8 years ago

This is only an issue with python 3 I think. I've got a fixed version in test at the moment and I'll put a pull request in at some point.

hkraal commented 8 years ago

Correct, it has been patched as it was a security issue for something. I was thinking about implementing _hash() method on the APICache class for a generic solution. At this moment there are just too many locations where hash() is being called which breaks the DRY principle.

jonobrien commented 8 years ago

it could definitely use a little refactoring :+1:

wtfrank commented 8 years ago

yeah I put a hash method on the base class.

TBH those caches should be pulled out into their own python file(s) at some point.

hkraal commented 8 years ago

Perfect!

Local import without package requirements is not cool and user error prone. Might be an good use case for namespace packages or something. Certainly something I plan on revisiting later on.

wtfrank commented 8 years ago

yeah namespace packages would solve the issue about the memcached dependency