search

spring-cloud / spring-cloud-circuitbreaker

Spring Cloud Circuit Breaker API and Implementations
Apache License 2.0
328 stars 109 forks source link

Option to disable the thread pool/executor service for Circuitbreaker #148

Closed kkotamar closed 2 years ago

kkotamar commented 2 years ago

This issue is to fix https://github.com/spring-cloud/spring-cloud-circuitbreaker/issues/147 I am trying to merge this 2.1.x branch since we are using 2.1.x version.

codecov[bot] commented 2 years ago

Codecov Report

Merging #148 (8eb62ed) into 2.1.x (1b48f80) will increase coverage by 0.96%. The diff coverage is 100.00%.

@@             Coverage Diff              @@
##              2.1.x     #148      +/-   ##
============================================
+ Coverage     78.10%   79.06%   +0.96%     
- Complexity       85       91       +6     
============================================
  Files            14       14              
  Lines           370      387      +17     
  Branches         13       15       +2     
============================================
+ Hits            289      306      +17     
  Misses           77       77              
  Partials          4        4
Impacted Files Coverage Δ
...er/resilience4j/Resilience4JAutoConfiguration.java 50.00% <ø> (ø)
...eaker/resilience4j/Resilience4JCircuitBreaker.java 70.00% <100.00%> (+5.29%) :arrow_up:
...esilience4j/Resilience4JCircuitBreakerFactory.java 82.00% <100.00%> (+2.93%) :arrow_up:
...ilience4j/Resilience4JConfigurationProperties.java 82.35% <100.00%> (+5.42%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1b48f80...8eb62ed. Read the comment docs.

kkotamar commented 2 years ago

I did this since it is only last step of execution that differs either execute in seperate pool or not. Creating another implementation will also have exact same code except last step.

ryanjbaxter commented 2 years ago

I agree it will work the current way, but I would rather use the approach of the presence of the executor service deciding the behavior

kkotamar commented 2 years ago

I made changes. Please review.

kkotamar commented 2 years ago

@ryanjbaxter Responded to comments. Added constructor for Resilience4JCircuitBreaker with out executor service.

kkotamar commented 2 years ago

@ryanjbaxter Addressed the comment and added new constructor.

kkotamar commented 2 years ago

@ryanjbaxter If this PR looks, can you merge and release?

kkotamar commented 2 years ago

@ryanjbaxter Addressed comments.

kkotamar commented 2 years ago

Docs are added.