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

Simplify Mongo client #6720

Closed
wants to merge 1 commit into from
Closed

Conversation

geoand
Copy link
Contributor

@geoand geoand commented Jan 22, 2020

MongoClientBuildItem is not used anywhere neither
are the RuntimeValue values produced by the recorder

MongoClientBuildItem is not used anywhere neither
are the RuntimeValue values produced by the recorder
launchMode.getLaunchMode(), shutdown,
codecs.getCodecProviderClassNames());
RuntimeValue<ReactiveMongoClient> reactiveClient = recorder.configureTheReactiveClient();
return new MongoClientBuildItem(client, reactiveClient);
Copy link
Member

Choose a reason for hiding this comment

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

That allows another extension to get the client. Why removing this feature?
Not sure if Panache Mongo requires it But I had something using this (for demo purpose, so no problem to break it).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mongo Panache is not using this (at least as far as I could tell and when I ran the tests locally nothing failed).

I removed it because nothing was using it and the implementation threw me off. If you feel it's valid to keep it around for future use, then I have absolutely no problem closing this PR

@loicmathieu
Copy link
Contributor

MongoDB with Panache didn't use it but maybe it should to be sure that the client is initialized before it. For the record, HIbernate with Panache uses the HibernateEnhancersRegisteredBuildItem as a marker build item.

This is also changed by the PR that provides multiple MongoDB Client support : #4529 ... better not merging this before it as it would need again a rebased ...

@geoand
Copy link
Contributor Author

geoand commented Jan 22, 2020

Yeah, let's see what #4529 does and I'll update this PR accordingly (or close it)

@gsmet
Copy link
Member

gsmet commented Jan 22, 2020

I had a request to expose the ValidatorFactory like that for Camel. So I think this is indeed useful and should probably be used by Camel.

@geoand
Copy link
Contributor Author

geoand commented Jan 22, 2020

I'll check with Camel as well. If it's not used, then I don't really see the point of keeping it around

@gsmet
Copy link
Member

gsmet commented Jan 29, 2020

Well, it might be a good idea to check if they would be interested and if we should generalize things like that instead of removing them.

@geoand
Copy link
Contributor Author

geoand commented Jan 30, 2020

That's a good idea. I'll see what's up on that side of things

@cescoffier
Copy link
Member

This pattern lets us manage the "I'm using Vert.x before you" issue we had when injecting Vert.x instances or the router instances. So, I believe it should be generalized. It does not cost much and allows extensions to be called once their dependencies are ready.

@geoand
Copy link
Contributor Author

geoand commented Feb 7, 2020

Closing this as Camel uses it

@geoand geoand closed this Feb 7, 2020
@gsmet gsmet added the triage/invalid This doesn't seem right label Feb 14, 2020
@geoand geoand deleted the simplify-mongo branch May 9, 2020 08:12
@snowdrop-bot snowdrop-bot mentioned this pull request May 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/mongodb triage/invalid This doesn't seem right
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants