-
Notifications
You must be signed in to change notification settings - Fork 190
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
two step circuit breaker #6
Conversation
gobreaker.go
Outdated
@@ -146,6 +153,13 @@ func NewCircuitBreaker(st Settings) *CircuitBreaker { | |||
return cb | |||
} | |||
|
|||
// NewTwoStep returns a new TwoStepCircuitBreaker configured with the given Settings. |
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.
I think it's better to rename NewTwoStep
to NewTwoStepCircuitBreaker
.
gobreaker_test.go
Outdated
@@ -52,11 +62,46 @@ func fail(cb *CircuitBreaker) error { | |||
return err | |||
} | |||
|
|||
func fail2Step(cb *TwoStepCircuitBreaker) { |
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.
I think fail2Step should return error
.
gobreaker_test.go
Outdated
@@ -211,6 +239,61 @@ func TestCustomCircuitBreaker(t *testing.T) { | |||
assert.Equal(t, StateChange{"cb", StateHalfOpen, StateClosed}, stateChange) | |||
} | |||
|
|||
func TestMultiStepBreaker(t *testing.T) { |
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.
I think it's better to rename TestMultiStepBreaker
to TestTwoStepCircuitBreaker
.
gobreaker_test.go
Outdated
@@ -211,6 +239,61 @@ func TestCustomCircuitBreaker(t *testing.T) { | |||
assert.Equal(t, StateChange{"cb", StateHalfOpen, StateClosed}, stateChange) | |||
} | |||
|
|||
func TestMultiStepBreaker(t *testing.T) { | |||
cb := &TwoStepCircuitBreaker{cb: defaultCB} |
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.
It's better to use NewTwoStepCircuitBreaker, which improves code coverage.
- renamed twostep breaker constructor - fixed failure test call to return an error
providing a two-step circuit breaker where checking whether a request can proceed and reporting the otucome is separated.
see previous discussions on the topic in #4