-
Notifications
You must be signed in to change notification settings - Fork 322
feat: drop cachetools
dependency in favor of simple local implementation
#1590
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
base: main
Are you sure you want to change the base?
Conversation
9d4fb8d
to
9ca2c14
Compare
Thank you @akx. Is using the standard library a reasonable alternative? https://docs.python.org/3/library/functools.html#functools.lru_cache |
@clundin25 Unfortunately not. The transparency of a |
@clundin25 Is there anything I can do to help this move along? |
How big is the change to update the client code so it can use the standard library and avoid maintaining this implementation? /cc @sai-sunder-s |
9ca2c14
to
e2364a3
Compare
Sorry, I wasn't sure whether that was directed at me or the person cc'd. Not trivial, since it depends on being able to introspect the cache entry before deciding whether it's usable. token, expiry = self._cache.get(audience, (None, None))
if token is None or expiry < _helpers.utcnow(): |
e2364a3
to
6eb667d
Compare
Rebased. Is there anything I can do to help this move along? cc @westarle, @clundin25. |
Rebased again. Could this be moved along? |
It starts to be more useful now, as there is new major update of cachetools 6.0.0 and it's not possible to use it together with current |
Rebased. Anything I can do to move this along (cc e.g. @clundin25)? |
This PR drops the dependency on
cachetools
, which was only used for its LRU cache class, in favor of a simple local implementation.This should have a small but positive effect on many downstream users given how many times this library is downloaded per day.