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

Add a MongoDB client extension #3022

Merged
merged 14 commits into from
Jul 10, 2019

Conversation

cescoffier
Copy link
Member

This is the first version of the Mongo extension.

It:

  • works in both JVM and native mode
  • proposes an imperative client (regular mongo client) and a reactive one (based on the reactive streams mongo driver)
  • supports authentication and TLS (even in native mode), but does not support mongo+srv:// in native mode (SVM limitation)
  • supports the hot reload

@cescoffier cescoffier added this to Waiting for merge in Quarkus 1.0.0 Planning via automation Jul 1, 2019
@cescoffier cescoffier force-pushed the features/mongo-client-extension branch 2 times, most recently from 6de4de9 to 8c5145c Compare July 2, 2019 08:51
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

I made a big review of the documentation part, NOT the code.

I think we should try to be consistent about the MongoDB naming but I let you decide what you prefer.

docs/src/main/asciidoc/mongo-guide.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/mongo-guide.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/mongo-guide.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/mongo-guide.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/mongo-guide.adoc Show resolved Hide resolved
docs/src/main/asciidoc/mongo-guide.adoc Show resolved Hide resolved
docs/src/main/asciidoc/mongo-guide.adoc Show resolved Hide resolved
docs/src/main/asciidoc/mongo-guide.adoc Show resolved Hide resolved
docs/src/main/asciidoc/mongo-guide.adoc Show resolved Hide resolved
@cescoffier
Copy link
Member Author

Ok, I believe we reached a milestone. I'm going to re-organized my commits and ask for a review.

@cescoffier cescoffier force-pushed the features/mongo-client-extension branch from 9929df5 to 40cf523 Compare July 3, 2019 08:52
@cescoffier cescoffier added this to the 0.19.0 milestone Jul 3, 2019
@cescoffier cescoffier marked this pull request as ready for review July 3, 2019 08:52
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

I added another round of comments.

I still think there are too many commits. Typically ac046f2 should be squashed as it really has no value.

@cescoffier cescoffier force-pushed the features/mongo-client-extension branch from 84d183c to 589ddc1 Compare July 5, 2019 14:08
@cescoffier
Copy link
Member Author

I've reduced the number of commits.

@gsmet gsmet changed the title mongo client extension Add a MongoDB client extension Jul 9, 2019
@gsmet
Copy link
Member

gsmet commented Jul 9, 2019

@cescoffier could you rebase, there are some conflicts with the pom files. It should be simple enough to fix.

@@ -0,0 +1,290 @@
package io.quarkus.mongodb.impl;
Copy link
Member

Choose a reason for hiding this comment

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

The convention until now was to have the impl classes in the runtime package so it should be io.quarkus.mongodb.runtime.

Copy link
Member Author

Choose a reason for hiding this comment

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

This module exposes an API, it would be weird to be in a runtime quarkus-specific structure. That being said, for implementation, I don't see an issue.

Copy link
Member

Choose a reason for hiding this comment

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

Well, this is the convention we decided. I wanted impl/spi packages but got ruled out.

So right now, the conventions we have are:
io.quarkus.<extension>: the potential runtime API
io.quarkus.<extension>.runtime: everything that is not API (you can have an impl subpackage here if you want)
io.quarkus.<extension>.deployment: everything from the deployment artifact

@cescoffier
Copy link
Member Author

Rebase done.

cescoffier and others added 11 commits July 10, 2019 09:46
…and feature coverage in native mode.

Inject both reactive and imperative client
Extend configuration support for both the imperative and reactive clients
Add an integration test for the reactive client
Fix credential and write concern configuration
Remove the new for having quarkus-reactive-streams-operators as a dependency of projects using the mongo client
Re-inject the client so it get the new config
Close the client during the hot reload so we don't have a connection shortage
Register SSL support if not explicitly disabled
Provide a substitution for the DNS lookup. However it won't work until Substrate provide a non-blocking multicast implementation.
Declare the mongo version and the mongo extension in the bom.
Fix integration tests

It works better when you start the database, weird, isn't it?
Also align with the embedded mongo used in the runtime tests
Also, the number of databases created by default differs on Mac and Linux. So, reduce the number of documents as on Windows the number of connection is limited to 500.
Co-Authored-By: Guillaume Smet <guillaume.smet@gmail.com>
This includes:
* module rename (artifactId, name...)
* package rename
* directory rename

Use duration instead of time in milliseconds.

Rename MongoDB directories, artifactIds, project name and package name

Replace SLF4J with JBoss Logging
@gsmet gsmet force-pushed the features/mongo-client-extension branch from a0bd6cf to 0f9a0a9 Compare July 10, 2019 08:19
@gsmet
Copy link
Member

gsmet commented Jul 10, 2019

I properly rebased to avoid the merge commit as it makes the history harder to read and adjusted a few things.

I will wait for CI and merge.

@rsvoboda
Copy link
Member

Thanks, just noticed that quarkus-quickstarts are broken due to missing quarkus-mongodb-client dependency for using-mongodb-client qs

@gsmet
Copy link
Member

gsmet commented Jul 10, 2019

@rsvoboda yes, I'm waiting for CI before merging this one.

@gsmet gsmet merged commit aa4e5e9 into quarkusio:master Jul 10, 2019
Quarkus 1.0.0 Planning automation moved this from Waiting for merge to Done Jul 10, 2019
@gsmet
Copy link
Member

gsmet commented Jul 10, 2019

And merged! Thanks @cescoffier and @loicmathieu !

@cescoffier cescoffier deleted the features/mongo-client-extension branch January 21, 2020 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants