Skip to content
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

[MAJOR] Update to R4j 1.1.0, and implement Retries #7

Merged
merged 4 commits into from
Oct 28, 2019

Conversation

breischl
Copy link
Collaborator

@breischl breischl commented Oct 25, 2019

I started out trying to implement support for R4j's Retry feature, which is in here. But I also bumped us up to Resilience4j v1.1.0, which is a pretty major change from the previous version. Therefore this is a breaking change from the previous version.

Added

  • Support for R4j Retry feature.

Changed

  • Updated to R4j v1.1.0, which is pretty majorly breaking from previous versions
  • Updated configs for CircuitBreakers to match new version of R4j, which is also breaking from previous configs (ie, some YAML configs that worked before won't anymore)
  • Update README.MD

Deleted

  • nothing

PR Checklist Forms
CHANGELOG.md updated
Unit test(s) added
Reviewer assigned
PR assigned (presumably to submitter)
Labels added (enhancement, bug, documentation)

@@ -6,7 +6,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/).

## [Unreleased]
### Changed
- nothing yet
- Added support for R4j `Retry`
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be moved to an ### Added section?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yep, done

builder.enableAutomaticTransitionFromOpenToHalfOpen();
}
@JsonProperty
private Class[] ignoreExceptions = new Class[0];
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we match the signature from resilience4j? (i.e. Class<? extends Throwable>[])

The same for the property below.

* Names of exceptions to ignore
*/
@JsonProperty
private Class[] ignoreExceptions = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Here and below are other spots where they could be Class<? extends Throwable>[].

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done for all Class[]

@eocantu eocantu self-assigned this Oct 28, 2019
@eocantu eocantu merged commit fd26396 into ExpediaGroup:master Oct 28, 2019
@breischl breischl deleted the update-and-retries branch October 28, 2019 20:53
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.

None yet

2 participants