Add cached property to the response#54
Add cached property to the response#54mtzfederico wants to merge 6 commits intoipinfo:masterfrom mtzfederico:master
Conversation
UmanShahzad
left a comment
There was a problem hiding this comment.
@mtzfederico thank you for the PR. I'd like to actually know what use case this serves, because currently it's not clear to me. I would imagine that if it needs to be known that a certain IP is in the cache or not, it'd be better to use the cache interface directly to do that.
README.md
Outdated
|
|
||
| In-memory caching of `details` data is provided by default via the [cachetools](https://cachetools.readthedocs.io/en/latest/) library. This uses an LRU (least recently used) cache with a TTL (time to live) by default. This means that values will be cached for the specified duration; if the cache's max size is reached, cache values will be invalidated as necessary, starting with the oldest cached value. | ||
|
|
||
| You can access the cached property to see if the response was cached. |
There was a problem hiding this comment.
Two nits here:
- Add a newline between the codeblock and this line.
- Convert the word "cached" here to an inline codeblock, i.e. "You can access the
cachedproperty..." This clearly distinguishes it as referring to something in the code.
ipinfo/handler.py
Outdated
| cache_key, | ||
| ) | ||
| from . import handler_utils | ||
| from ipinfo import details |
|
I find it useful for some simple statistics and for debugging, but you mention that it would be easier to use the cache interface. I am not very familiar with cachetools, it might be easier to just add some sample code for debugging to the docs. I am not sure about that import, I just ran my tests again without it and it works. My guess is that my editor added it automatically. |
|
@mtzfederico You wouldn't have to work directly with You can access/update the cache via dictionary-like notation, but the cache key must be gotten via >>> import ipinfo
>>> from ipinfo.handler_utils import cache_key
>>> h = ipinfo.getHandler()
>>> ip_cache_key = cache_key('1.1.1.1')
# __contains__
>>> ip_cache_key in h.cache
# __getitem__
>>> handler.cache[ip_cache_key]
# __setitem__
>>> h.cache[ip_cache_key] = NoneIf this is sufficient I'd much rather we just make a PR explaining this, rather than adding a new field which is really only metadata. |
|
|
||
| In-memory caching of `details` data is provided by default via the [cachetools](https://cachetools.readthedocs.io/en/latest/) library. This uses an LRU (least recently used) cache with a TTL (time to live) by default. This means that values will be cached for the specified duration; if the cache's max size is reached, cache values will be invalidated as necessary, starting with the oldest cached value. | ||
|
|
||
| You can access the cached property to see if the response was `cached`. |
There was a problem hiding this comment.
The wrong cached was wrapped with '`'; in the example at #54 (comment) I wrapped only the first one.
|
@UmanShahzad Thanks for your examples. I'm going to close this PR and create another one with the examples that you provided. Should I put it in the README.md below the "Using a different cache" section? |
|
@mtzfederico that would be great, thank you. Yes you can make a new section below that as: #### Accessing the cache directly
... |
This PR adds a cached property to the response. This lets you check if the response was cached or not.