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

Allow access to the entry (and its expiration) via get() #8

Closed
wants to merge 1 commit into from
Closed

Allow access to the entry (and its expiration) via get() #8

wants to merge 1 commit into from

Conversation

nickbp
Copy link

@nickbp nickbp commented Jul 21, 2021

Adds an entry() accessor, which operates in parallel to the existing deref() access to the underlying value.

(Resolves #6)

@nickbp nickbp changed the title Allow access to the entry (and its expiration) Allow access to the entry (and its expiration) via get() Jul 21, 2021
@nickbp
Copy link
Author

nickbp commented Jan 26, 2022

Ping if you're around. Been using this version for the last few months and it's been working well.

Adds an `entry()` accessor, which operates in parallel to the existing `deref()` access to the underlying value.
@nickbp
Copy link
Author

nickbp commented Feb 11, 2022

In order to unblock publishing other packages to crates.io that use this patch, I've created a retainer_nickbp crate with these changes.

@whitfin
Copy link
Owner

whitfin commented Mar 17, 2022

@nickbp what exactly is the use case behind accessing the entry itself?

Edit: nvm I saw the attached issue. Let me think about it a little and I'll get back to you after I try a few things out. Your changes work but I feel there may be a better option.

@nickbp
Copy link
Author

nickbp commented Mar 17, 2022

Example usage in a DNS server here: https://git.sr.ht/~nickbp/originz/tree/main/item/src/cache/memory.rs#L53

If the entry is present, its remaining TTL in the cache is included in the DNS response

@whitfin
Copy link
Owner

whitfin commented Mar 17, 2022

Okay @nickbp, played around with it a bit...

I think it is a mistake that CacheEntryReadGuard<'a, V> currently derefs to V, it should probably deref to CacheEntry<V>, which would give you what you need. If you want to deref to V you can then just **guard.

What do you think about this approach?

@nickbp
Copy link
Author

nickbp commented Mar 17, 2022

Sounds good to me!

@whitfin
Copy link
Owner

whitfin commented Mar 20, 2022

@nickbp after playing around with the various options, I have decided to mask CacheEntry<V> from the public API entirely, in favour of just CacheReadGuard<V> (which is just a rename of CacheEntryReadGuard<V>. This struct exposes both the expiration and value, and derefs to the value directly as before.

I stripped CacheEntry<V> out of return types too, which I don't think should really cause any issues (why would you care about an expiration after an entry is removed, for example?). Anything we want to surface in future can just live on the guard itself.

In a couple of places I use this crate, the usage actually cleaned up a little bit with these changes. Hopefully you can test and notice the same, while also getting access to the expiration values you needed. Please let me know if everything looks good, then I can merge and push it up!

I still have a few things I'm looking at for making it smoother, but I don't expect the public API to change much (or at all).

@whitfin
Copy link
Owner

whitfin commented Mar 22, 2022

I have published the changes in 0.3.0; I can follow up with any changes that might be required in future.

@whitfin whitfin closed this Mar 22, 2022
@nickbp
Copy link
Author

nickbp commented Mar 31, 2022

This works great, thanks a ton!

@nickbp nickbp deleted the master branch March 31, 2022 21:05
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

Successfully merging this pull request may close these issues.

Feature: get() function that returns expiration info?
2 participants