Skip to content

Create a new Vert.x instance only if one doesn't exist already #1003

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Oct 18, 2021
Merged

Create a new Vert.x instance only if one doesn't exist already #1003

merged 2 commits into from
Oct 18, 2021

Conversation

DavideD
Copy link
Member

@DavideD DavideD commented Oct 16, 2021 •
edited
Loading

Fixes #1001
Superseeds #1002

Thanks to @codingxu97 fix, we don't need to override the VertxInstance for the tests

codingxu97 and others added 2 commits October 16, 2021 16:43
@codingxu97 @DavideD
[hibernate#1001] Don't create a new Vert.x instance if one exists
21ddc12 
DefaultVertxInstance used to create a new Vert.x instance
every time, even if one was already present.

This commit make it easier to use Hibernate Reactive when
a Vert.x instance already exists. In particular, when used
with Vert.x verticles.
@DavideD
[hibernate#1001] Remove setup of custom VertxInstance for tests
ede99c0 
It's not required anymore because now we get the
Vert.x instance from the current context.
@DavideD DavideD changed the title 1001 default vertx instance fixed Create a new Vert.x instance only if one doesn't exist already Oct 16, 2021
@DavideD DavideD requested a review from Sanne October 16, 2021 15:48
@DavideD
Copy link
Member Author

DavideD commented Oct 16, 2021

I don't know if I should merge this before the final but it shouldn't cause any problem:
for somebody that already overrides the service, nothing will change.
In the other case a new Vert.x instance gets created anyway.

@codingxu97
Copy link
Contributor

I don't know if I should merge this before the final but it shouldn't cause any problem: for somebody that already overrides the service, nothing will change. In the other case a new Vert.x instance gets created anyway.

It will not affect the previous implementation IMHO. As you told, this PR just to avoid duplicate creation of Vertx instance, this is the design principles of Vert.x, it will be broken without existence check.

@DavideD
Copy link
Member Author

DavideD commented Oct 17, 2021

Ok, Quarkus overrides the default VertxInstance service anyway and I've run the tests for the hibernate reactive extension and everything seems to work fine.

I will merge this tomorrow

@gavinking
Copy link
Member

Looks fine to me.

@DavideD DavideD merged commit 75454d5 into hibernate:main Oct 18, 2021
@DavideD
Copy link
Member Author

DavideD commented Oct 18, 2021

Merged! Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Sanne Sanne Awaiting requested review from Sanne

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

DefaultVertxInstance should not create a new Vertx instance if one exists already
3 participants
@DavideD @codingxu97 @gavinking