Feature discussions

[jayrbolton]

I think what we have already is good to publish. But here are some further thoughts:

It would be nice to be able to pass the cache_id as a parameter, rather than saving it to the class. This way, you can more easily deal with multiple cache files:

cache_id1 = cache_client.generate_cacheid({'stuff': 'things'})
cache_id2 = cache_client.generate_cacheid({'foo': 'bar'})
cache_client.download_cache(cache_id1, '/my_file1.txt')
cache_client.download_cache(cache_id2, '/my_file2.txt')

A specific error for a nonexistent cache might be more useful than ValueError. This could be used to do a try/except on the download:

try:
   cache_client.download_cache(cache_id, '/my_file.txt')
except CacheNonexistent:
   file_path = expensively_generate_file('/my_file.txt')
   cache_client.upload_cache(cache_id, 'my_file.txt')

Or we could even shortcut the above with a utility such as:

fetch_cache('/my_file.txt', my_cache_params, expensive_file_generator)

Where expensive_file_generator is a function that returns the file path to upload to the cache. This function only gets run if we cannot download the cache. The my_cache_params object would be passed to the function. It would also be used to generate the cache id.

[rroutsong]

It would be nice to be able to pass the cache_id as a parameter, rather than saving it to the class. This way, you can more easily deal with multiple cache files:

This looks like a better solution, and I will start implementing this. When generating new cache ids, would probably require two requests one to /download and parse json make sure cache_id doesn't already exist, my thought here was handling the cache id internally to prevent cache_id overlap.

I will also move all the exceptions to a seperate files and write this custom non-existent cache exception.

[rroutsong]

@jayrbolton

I think addressed all the concerns here and made some clean up improvements. Let me know if you want me to change anything.

Ryan