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

Improvements #71

Merged
merged 6 commits into from
Sep 6, 2023
Merged

Improvements #71

merged 6 commits into from
Sep 6, 2023

Conversation

Ladicek
Copy link
Contributor

@Ladicek Ladicek commented Jul 11, 2023

Motivation:

The context is vert-x3/issues#633, and this commit is a first step towards that goal. It does:

  • remove all the Hystrix stuff, because Hystrix is long dead
  • improve documentation and implementation
  • update test-scoped dependencies

I have also started working on a Resilience4j howto, which shall be linked from the documentation here. In fact, I already have Vert.x wrappers for all Resilience4j fault tolerance patterns (bulkhead, circuit breaker, rate limiter, retry, time limiter -- I didn't include cache for obvious reasons), so we could possibly make into a full-fledged extension (and deprecate the Vert.x circuit breaker maybe?). That's for later, for sure.

Conformance:

Your commits should be signed and you should have signed the Eclipse Contributor Agreement as explained in https://github.com/eclipse/vert.x/blob/master/CONTRIBUTING.md
Please also make sure you adhere to the code style guidelines: https://github.com/vert-x3/wiki/wiki/Vert.x-code-style-guidelines

@vietj vietj added this to the 5.0.0 milestone Jul 12, 2023

The fallback receives a:

* {@link io.vertx.circuitbreaker.OpenCircuitException} when the circuit breaker is opened
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this section about reported exception deleted ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

@vietj vietj left a comment

Choose a reason for hiding this comment

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

minor concern about the exception part deleted

@Ladicek
Copy link
Contributor Author

Ladicek commented Jul 14, 2023

Added one more commit that mentions Resilience4j at the end of the documentation (where Hystrix usage used to be documented). The documentation is very brief, it mostly just points to the recently published how-to: https://how-to.vertx.io/resilience4j-howto/

@tsegismont
Copy link
Contributor

I have also started working on a Resilience4j howto, which shall be linked from the documentation here. In fact, I already have Vert.x wrappers for all Resilience4j fault tolerance patterns (bulkhead, circuit breaker, rate limiter, retry, time limiter -- I didn't include cache for obvious reasons), so we could possibly make into a full-fledged extension (and deprecate the Vert.x circuit breaker maybe?). That's for later, for sure.

Another option would be a Reactiverse project for Resilience4j integration

@Ladicek
Copy link
Contributor Author

Ladicek commented Jul 18, 2023

That would make sense too, good point!

@vietj
Copy link
Contributor

vietj commented Sep 5, 2023

can you rebase this PR please @Ladicek ?

The documentation used to describe how to use Hystrix with Vert.x.
Since Hystrix has been obsolete for a long time and the documentation
was removed in a recent commit, this commit adds a modern replacement:
Resilience4j. The documentation mainly links to the Resilience4j
Vert.x how-to that was published recently.
@Ladicek
Copy link
Contributor Author

Ladicek commented Sep 5, 2023

Rebased.

@vietj vietj merged commit 0ecd7e1 into vert-x3:master Sep 6, 2023
4 checks passed
@vietj
Copy link
Contributor

vietj commented Sep 6, 2023

Thanks @Ladicek

@Ladicek Ladicek deleted the improvements branch September 6, 2023 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants