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.

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

Initialize hibernate reactive before gRPC #24420

Merged
merged 1 commit into from
Mar 22, 2022
Merged

Initialize hibernate reactive before gRPC #24420

merged 1 commit into from
Mar 22, 2022

Conversation

michalszynkiewicz
Copy link
Member

No description provided.

@quarkus-bot quarkus-bot bot added area/hibernate-reactive Hibernate Reactive area/persistence OBSOLETE, DO NOT USE labels Mar 18, 2022
@michalszynkiewicz michalszynkiewicz mentioned this pull request Mar 18, 2022
Merged
michalszynkiewicz
michalszynkiewicz commented Mar 18, 2022
View reviewed changes
...yment/src/main/java/io/quarkus/hibernate/reactive/deployment/HibernateReactiveProcessor.java Outdated
@@ -183,6 +185,7 @@ void waitForVertxPool(List<VertxPoolBuildItem> vertxPool,

@BuildStep
@Record(RUNTIME_INIT)
@Produce(SyntheticBeansRuntimeInitBuildItem.class)
Copy link
Member Author

Choose a reason for hiding this comment

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

@mkouba grpc is initialized after this build item, and it sounds like something that should be safe ;) Do you know if there is build item that could be to express this order?

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't want to have explicit dependency between HR and gRPC.

Copy link
Member

@gsmet gsmet Mar 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Not sure if it's useful in this case but for some other similar purposes we are producing a ServiceStartBuildItem in Hibernate ORM.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think I can use it, the GrpcServerProcessor step that I want to make sure runs after this step is producing ServiceStartBuildItem itself.

Copy link
Contributor

Choose a reason for hiding this comment

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

@michalszynkiewicz You can't produce a SyntheticBeansRuntimeInitBuildItem. It can be only used to make sure a build step is executed AFTER the RUNTIME_INIT synthetic beans are initialized.

ServiceStartBuildItem, on the other hand, is used to make sure a build step is exectued before the StartupEvent is fired.

@michalszynkiewicz
Copy link
Member Author

FTR, the discussion on how to fix this got moved to #24441 and #24439

@gsmet
Copy link
Member

gsmet commented Mar 21, 2022

This starts to look like a choose-your-own-adventure book :).

@michalszynkiewicz
Copy link
Member Author

This starts to look like a choose-your-own-adventure book :).

yep :D

I'll try to propose a PR with #24441 tomorrow or today if I have a while

@gsmet
Copy link
Member

gsmet commented Mar 21, 2022

@michalszynkiewicz let me try to switch to synthetic beans first as my understanding is that it will fix the issue.

@michalszynkiewicz michalszynkiewicz force-pushed the init-hibernate-reactive-before-synthetic-beans branch from 3083e25 to 9a38fba Compare March 21, 2022 19:51
@quarkus-bot quarkus-bot bot added area/arc Issue related to ARC (dependency injection) area/grpc gRPC area/hibernate-orm Hibernate ORM labels Mar 21, 2022
@michalszynkiewicz
Copy link
Member Author

I changed the PR to use a dedicated build item

@michalszynkiewicz
Copy link
Member Author

Quarkus CI / Native Tests - Messaging1 (pull_request) failure is a failure to start a container - not related to this change

@quarkus-bot

This comment has been minimized.

Sign in to view
@michalszynkiewicz michalszynkiewicz changed the title Initialize hibernate reactive before synthetic beans to enforce init before gRPC Initialize hibernate reactive before gRPC Mar 22, 2022
@michalszynkiewicz
Copy link
Member Author

@mkouba @gsmet I think it would be good to get this one in the CR1, so would appreciate feedback so that I can rework it if needed

mkouba
mkouba reviewed Mar 22, 2022
View reviewed changes
extensions/grpc/deployment/src/main/java/io/quarkus/grpc/deployment/GrpcServerProcessor.java Outdated
@@ -457,6 +458,7 @@ void gatherGrpcInterceptors(BeanArchiveIndexBuildItem indexBuildItem,
@BuildStep
@Record(value = ExecutionTime.RUNTIME_INIT)
@Consume(SyntheticBeansRuntimeInitBuildItem.class)
@Consume(BeanInitializedBuildItem.class)
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, how does @Consume behave in case of a MultiBuildItem? I mean is the behavior equivalent to a List<BeanInitializedBuildItem> param?

CC @dmlloyd

@michalszynkiewicz michalszynkiewicz force-pushed the init-hibernate-reactive-before-synthetic-beans branch from 9a38fba to 5e99256 Compare March 22, 2022 13:03
@michalszynkiewicz michalszynkiewicz force-pushed the init-hibernate-reactive-before-synthetic-beans branch from 5e99256 to 99a86ae Compare March 22, 2022 13:30
@mkouba mkouba mentioned this pull request Mar 22, 2022
Closed
@gsmet gsmet merged commit 2e52498 into quarkusio:main Mar 22, 2022
@quarkus-bot quarkus-bot bot added this to the 2.8 - main milestone Mar 22, 2022
@mkouba mkouba mentioned this pull request Apr 7, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gsmet gsmet gsmet left review comments

@mkouba mkouba mkouba approved these changes

Assignees
No one assigned
Labels
area/arc Issue related to ARC (dependency injection) area/grpc gRPC area/hibernate-orm Hibernate ORM area/hibernate-reactive Hibernate Reactive area/persistence OBSOLETE, DO NOT USE
Projects
None yet
Milestone
2.8.0.CR1
Development

Successfully merging this pull request may close these issues.
3 participants
@michalszynkiewicz @gsmet @mkouba