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

Seperate afterDeploymentValidation into two submethods #2357

Merged
merged 1 commit into from
Nov 2, 2023

Conversation

benjamin-confino
Copy link
Contributor

Hello

I have been working on getting reactive messaging to work with InstantOn in open liberty. I have a proof of concept and have determined that if I prevent the line mediatorManager.start() in ReactiveMessagingExtension from running before the checkpoint, and instead run it after the restore, everything just works.

However because mediatorManager.start() is invoked as part of a larger method containing code that ideally will run before the checkpoint I had to modify that class locally. I would like to submit a simple change that makes ReactiveMessagingExtension.afterDeploymentValidation call two subordinate methods. Then in my code I can simply extend, rather than modify, ReactiveMessagingExtension and call the subordinate methods directly at the appropriate times.

Copy link
Contributor

@cescoffier cescoffier left a comment

Choose a reason for hiding this comment

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

The change looks good, but before merging, we need to see if it does change something in Quarkus. I don't believe it does (as we do not use this method if I remember correctly).

@ozangunalp do you know?

@Ladicek
Copy link
Collaborator

Ladicek commented Nov 2, 2023

I'm pretty sure the entire ReactiveMessagingExtension is ignored in Quarkus, because it's a CDI portable extension. The Quarkus extension for Reactive Messaging has to implement the entire logic on its own.

Copy link
Collaborator

@ozangunalp ozangunalp left a comment

Choose a reason for hiding this comment

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

This looks good to me too.
As long as MediatorManager start method is there there isn't any impact on Quarkus integration. Indeed the CDI Extension is not used in Quarkus

@ozangunalp ozangunalp merged commit 175cfff into smallrye:main Nov 2, 2023
@ozangunalp ozangunalp added this to the 4.11.0 milestone Nov 7, 2023
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.

4 participants