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

Export Neo4j driver as dedicated BuildItem. #11115

Merged

Conversation

michael-simons
Copy link
Contributor

This change allows other extensions to retrieve the driver during their deployment process and use it as dependency.

Background: I'm writing another extension and I need the driver in it. So my extension will depend on Neo4j. I thought Arc respectively CDI would also allow the Driver to be injected, but I'm pretty sure I'm wrong on that.

So with the dedicated Neo4jDriverBuildItem in addition to the CDI producer (both working on the same Driver value), the Neo4j extension provides the instance to the application and possible other extensions.

@machi1990 machi1990 requested a review from gsmet July 31, 2020 07:48
Neo4jConfiguration configuration,
ShutdownContextBuildItem shutdownContext) {

recorder.configureNeo4jProducer(beanContainerBuildItem.getValue(), configuration, shutdownContext);
Driver driver = recorder.initializeDriver(configuration, shutdownContext);
Copy link
Member

Choose a reason for hiding this comment

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

I don't think that will work, you need to use a RuntimeValue<Driver>. We have plenty of examples if you look for RuntimeValue in the code.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, this is absolutely correct. It might work even without it, but there is no point in risking it and not using RuntimeValue

Copy link
Contributor

Choose a reason for hiding this comment

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

Just to clarify why this worked without RuntimeValue: It worked because Driver is an interface and as such the bytecode recorder knew to just create a proxy for it.

If however the class was not an interface, the whole endeavor would have failed 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for getting back with this info. Might come in handy some time to know this!

Copy link
Contributor

Choose a reason for hiding this comment

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

You are welcome!

@michael-simons
Copy link
Contributor Author

michael-simons commented Jul 31, 2020 via email

@gsmet
Copy link
Member

gsmet commented Jul 31, 2020

Maybe I misunderstood something but I thought you had to wrap the elements you returned from the recorder in a RuntimeValue.

@geoand am I incorrect?

@michael-simons
Copy link
Contributor Author

michael-simons commented Jul 31, 2020 via email

@michael-simons
Copy link
Contributor Author

I updated the PR. Thanks again for your explanation.

While working on this (and also on the client using it) I have to say, your exception messages are great and really helpful getting those things right! 👍

@geoand
Copy link
Contributor

geoand commented Aug 3, 2020

Looks great, thanks!

Can you please squash the commits?

This change allows other extensions to retrieve the driver during their deployment process and use it as dependency. The driver is wrapped as  RuntimeValue<Driver> so that it can be safely passed between different runtime steps.
@michael-simons michael-simons force-pushed the feature/add_neo4j_build_item branch from 30446f5 to a2c95c2 Compare August 3, 2020 09:44
@gsmet gsmet merged commit d936363 into quarkusio:master Aug 5, 2020
@gsmet
Copy link
Member

gsmet commented Aug 5, 2020

Merged, thanks!

@gsmet gsmet added this to the 1.8.0 - master milestone Aug 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants