-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
Support white-list and black-list commands and keys #3740
Conversation
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## 5.2.0 #3740 +/- ##
============================================
- Coverage 75.54% 75.10% -0.44%
+ Complexity 4984 4981 -3
============================================
Files 300 303 +3
Lines 15241 15261 +20
Branches 1157 1163 +6
============================================
- Hits 11514 11462 -52
- Misses 3221 3286 +65
- Partials 506 513 +7 ☔ View full report in Codecov by Sentry. |
Should we use allow-list and deny-list? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just some naming related commends.
} | ||
|
||
@Override | ||
public boolean isCacheable(ProtocolCommand command, Object... keys) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that there is a default
in the interface, this is not needed.
@@ -74,7 +78,7 @@ public static RedisProtocol getRedisProtocol(URI uri) { | |||
|
|||
private static final Integer ZERO_INTEGER = 0; | |||
|
|||
public static ClientSideCache getClientSideCache(URI uri) { | |||
public static ClientSideCacheConfig getClientSideCache(URI uri) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a new argument, a cache config builder? Can be Function<ClientSideCache, ClientSideCacheConfig>
. That might be a breaking change, so it can be a new method, to have an alternative.
@sazzad16 LGTM. |
csc
package