fix: cleanup some abstractions for circuit breakers #71
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #71 +/- ##
============================================
- Coverage 60.93% 60.77% -0.17%
Complexity 193 193
============================================
Files 35 36 +1
Lines 750 752 +2
Branches 45 45
============================================
Hits 457 457
- Misses 263 265 +2
Partials 30 30
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| @@ -18,7 +18,7 @@ public static Map<String, CircuitBreakerConfig> getCircuitBreakerConfigs( | |||
| public static List<String> getDisabledKeys( | |||
| Map<String, CircuitBreakerThresholds> configurationMap) { | |||
| return configurationMap.entrySet().stream() | |||
| .filter(entry -> entry.getValue().isEnabled()) | |||
| .filter(entry -> !entry.getValue().isEnabled()) | |||
There was a problem hiding this comment.
I take no credit here - I saw this in https://github.com/hypertrace/java-grpc-utils/pull/70/files#diff-ed223a46181a1d76999c892bb320618ddd0643acdaf22b35ee36d56ef87dba96 :)
There was a problem hiding this comment.
The only approvable part of that PR 🤷♂️
| public class CircuitBreakerInterceptorFactory { | ||
| private final Clock clock; | ||
|
|
||
| CircuitBreakerInterceptor buildInterceptor(CircuitBreakerConfiguration<?> configuration) { |
There was a problem hiding this comment.
It should be static method right ? Should client create a object for this now ?
There was a problem hiding this comment.
Using a static here doesn't really provide benefits. For those using DI, they can inject the factory as long as clock is provided in the injector. For those without, it just becomes
new CircuitBreakerInterceptorFactory(clock).buildInterceptor(config);
instead of
CircuitBreakerInterceptorFactory.buildInterceptor(clock, config);
No description provided.