Skip to content

Conversation

ItalyPaleAle
Copy link
Contributor

This is another spin-off from #4903

It changes the resiliency.PolicyDefined method to return the actual policy object and not just a boolean indicating whether a policy exists.

This was necessary for #4903, but I co-authored this change with @halspang who said he thinks we need this for other things too, in the future.

spin-off from dapr#4903

Co-authored-by: Hal Spang <halspang@microsoft.com>
Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
@ItalyPaleAle ItalyPaleAle requested review from a team as code owners August 19, 2022 20:48
@codecov
Copy link

codecov bot commented Aug 19, 2022

Codecov Report

Merging #5050 (dc8dd73) into master (7d7c385) will increase coverage by 0.08%.
The diff coverage is 85.10%.

@@            Coverage Diff             @@
##           master    #5050      +/-   ##
==========================================
+ Coverage   65.32%   65.40%   +0.08%     
==========================================
  Files         114      114              
  Lines       12650    12684      +34     
==========================================
+ Hits         8263     8296      +33     
- Misses       3798     3799       +1     
  Partials      589      589              
Impacted Files Coverage Δ
pkg/actors/actors.go 58.65% <0.00%> (ø)
pkg/messaging/direct_messaging.go 18.75% <0.00%> (ø)
pkg/resiliency/noop.go 80.00% <0.00%> (ø)
pkg/resiliency/resiliency.go 74.79% <97.56%> (+1.65%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
@@ -840,6 +897,11 @@ func (e *circuitBreakerInstances) Remove(name string) {
e.Unlock()
}

// HasRetries returns true if the policy is configured to have more than 1 retry.
func (p PolicyDescription) HasRetries() bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any specific reason this has to be an exported function in this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is needed in #4903

Copy link
Contributor

@artursouza artursouza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you follow up with a PR renaming PolicyDefined() to GetPolicy()?

@artursouza artursouza merged commit b6f5b1e into dapr:master Aug 23, 2022
@ItalyPaleAle ItalyPaleAle deleted the return-resiliency-policy branch August 23, 2022 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants