search

sasa1977 / con_cache

ets based key/value cache with row level isolated writes and ttl support
MIT License
910 stars 71 forks source link

add :no_update option #16

Closed rixmann closed 8 years ago

rixmann commented 8 years ago

In %ConCache.Item{ttl: :no_update} can be set to omit touching the entries ttl even when updating/putting.

sasa1977 commented 8 years ago

Thanks for the pull.

Once I saw your mail, and glanced at the code, my idea was similar, but I'd do it in a slightly different way. A problem with doing this in the ConCache.Owner is that we're needlessly sending a message to the process, even though we could decide earlier that there's no need to update ttl.

So instead, I'd add a clause here which does nothing if ttl is :no_update.

Moreover, you need to adapt this typespec to indicate that ttl can be pos_integer | :no_update. We also need to document :no_update. And finally, we need some tests for this :-)

rixmann commented 8 years ago

updated :)

rixmann commented 8 years ago

i will use it this way for myself, test and documentation will be added later

sasa1977 commented 8 years ago

Looks better. I'll do the proper check sometimes this weekend. Thanks!

sasa1977 commented 8 years ago

OK, I took a closer look at the code, and it looks good. So we only need test, and a note in ConCache.Item that explains the new option.

sasa1977 commented 8 years ago

@rixmann I wanted to check the status of this. I'm basically happy with the change, but we're still missing tests and docs. If you don't have time for that, I could do it myself.

rixmann commented 8 years ago

i did not really check for the tests ... you used the wording "proper" check - so i thought there was some more sophisitcated property based checking ;)

should be fine now

sasa1977 commented 8 years ago

Great, thanks! I'll take a look later today.

sasa1977 commented 8 years ago

Thank you for contributing!