minimal working example with TTL #202
Conversation
Pull Request Test Coverage Report for Build 19915251842Details
💛 - Coveralls |
jmxpearson
left a comment
jmxpearson
left a comment
There was a problem hiding this comment.
Mostly looks good. I have a couple of questions in the review, and one line still needs to be reformatted to pass black checks.
| Args: | ||
| object: the object to store in Redis | ||
| object_key (str): the key under which the object should be stored | ||
| ex (int): TTL; the number of seconds after which the object got expired |
There was a problem hiding this comment.
English phrasing: Maybe something like
ex (int): time to live; number of seconds until object will expire
improv/store.py
Outdated
|
|
||
| self.client.set(object_key, pickle.dumps(object, protocol=5), nx=True) | ||
| self.client.set( | ||
| object_key, pickle.dumps(object, protocol=5), nx=True, ex=ex |
There was a problem hiding this comment.
nx = True means the key will only be set if it does not already exist in the redis store. If the key already exists, set() will be skipped.
There was a problem hiding this comment.
Ah we should probably decide if we want default behavior to be non-overwriting or overwriting. The keys should never conflict but in the very very unlikely case they do. I think perhaps overwriting is more real-time... @jmxpearson ?
There was a problem hiding this comment.
On the one hand, overwriting by default is begging for silent failure. On the other, speed is necessary.
In all, I’d probably prefer not overwriting as the default but exposing the option and documenting u der some section detailing optimizations (TTL, store size, etc.)?
There was a problem hiding this comment.
@paulawucu Let's keep nx=True then, but have it log if set is skipped? Is that possible with Redis code? Does it return any warnings?
Could also regen a key if set is going to be skipped.
Let's also file an issue to add any error handling for if there is key collision and the user is choosing nx=False.
| self.data[self.frame_num], str(f"Gen_raw: {self.frame_num}") | ||
| ) | ||
| else: | ||
| data_id = self.client.put(self.data[self.frame_num], ex=40) |
There was a problem hiding this comment.
Is this the only line we're changing relative to the other generator? Could we not inherit and overload the one method? Just seems like a lot of code duplication.
5 participants
Adding expiration of data during a put operation; results in much lower memory usage and much faster overall execution, in our use cases.