Conversation
Signed-off-by: Manish Kumar <manish.kumar1@india.nec.com>
Signed-off-by: Manish Kumar <manish.kumar1@india.nec.com>
|
@mk46 can you fix the formatting issues that were caught in CI? Also, can you take a look at our style guide and fix some of the deviations in Given that this is a pretty big change, it would also be nice to beef up the description to provide context for folks just show up (like me!). |
|
Also, why not just add https://github.com/google/jwt_verify_lib/tree/master/simple_lru_cache as an external dependency rather than copy the files? |
As of now, this is already used as an external dependency in Jwt Cache. The purpose of adding to envoy is to consumed by other modules of envoy. This code has a copyright of google. Please see https://github.com/google/jwt_verify_lib/blob/e5d6cf7067495b0868787e1fd1e75cef3242a840/simple_lru_cache/simple_lru_cache.h#L1 |
|
@htuch Any thought? |
|
I think the idea was that this might be useful beyond the JWT verifier. It seems there are a few other places that LRU might make sense (basically, whenever we're caching stuff, e.g. DNS, gRPC clients, etc.). Maybe do a quick survey and list all places LRU cache might make sense based on existing uses? |
|
Initially, I had tried to keep this code in envoy but as per discussion in #14341 (comment), moved Lru cache code in jwt_verify_lib. |
| namespace Envoy { | ||
| namespace LruCache { | ||
|
|
||
| #undef GOOGLE_DISALLOW_EVIL_CONSTRUCTORS |
There was a problem hiding this comment.
Please use the = delete ; syntax to make class noncopyable without the macro.
lizan
left a comment
lizan
left a comment
There was a problem hiding this comment.
please adjust coding style as well.
| @@ -0,0 +1,17 @@ | |||
| load( | |||
There was a problem hiding this comment.
IMO we can just put this in source/common/common
| void operator=(const TypeName&) | ||
|
|
||
| // Define number of microseconds for a second. | ||
| const int64_t kSecToUsec = 1000000; |
|
I am closing as of now, I have been busy with some other task. |
8 participants
Signed-off-by: Manish Kumar manish.kumar1@india.nec.com
Commit Message: Move LruCache from jwt_verify_lib/simple_lru_cache
Additional Description: This PR is follow up to comment:14341
Risk Level:
Testing: unit and build
Docs Changes: n/a
Release Notes: n/a
Fixes #16108