You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "garydgregory (via GitHub)" <gi...@apache.org> on 2024/04/18 14:15:02 UTC

[PR] Feature/2.x/mongodb next gg (logging-log4j2)

garydgregory opened a new pull request, #2486:
URL: https://github.com/apache/logging-log4j2/pull/2486

   Mongo DB PR with 3 commits
   - Add a new `log4j-mongodb` module to track the current MongoDB Driver, version 5
   - Deprecate `log4j-mongodb4` in favor of `log4j-mongodb`
   - Remove `log4j-mongodb3`
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Feature/2.x/mongodb next gg (logging-log4j2)

Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #2486:
URL: https://github.com/apache/logging-log4j2/pull/2486#discussion_r1572547482


##########
log4j-mongodb/src/test/java/org/apache/logging/log4j/mongodb/MongoDbResolver.java:
##########
@@ -142,11 +144,11 @@ private static final class MongoClientHolder implements CloseableResource, Suppl
         private final MongoClient mongoClient;
 
         public MongoClientHolder(final Mongod mongod, final TestProperties props) {
-            state = mongod.start(Version.Main.V3_6);
+            state = mongod.start(Version.Main.V4_4);

Review Comment:
   That did not even compile. That's what I get for using GH! ;-)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Feature/2.x/mongodb next gg (logging-log4j2)

Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory merged PR #2486:
URL: https://github.com/apache/logging-log4j2/pull/2486


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Feature/2.x/mongodb next gg (logging-log4j2)

Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #2486:
URL: https://github.com/apache/logging-log4j2/pull/2486#discussion_r1572509894


##########
pom.xml:
##########
@@ -257,8 +257,8 @@
     <module>log4j-jul</module>
     <module>log4j-layout-template-json</module>
     <module>log4j-layout-template-json-test</module>
-    <module>log4j-mongodb3</module>
     <module>log4j-mongodb4</module>
+    <module>log4j-mongodb</module>

Review Comment:
   Maven should reorder.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Feature/2.x/mongodb next gg (logging-log4j2)

Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #2486:
URL: https://github.com/apache/logging-log4j2/pull/2486#discussion_r1572459246


##########
pom.xml:
##########
@@ -257,8 +257,8 @@
     <module>log4j-jul</module>
     <module>log4j-layout-template-json</module>
     <module>log4j-layout-template-json-test</module>
-    <module>log4j-mongodb3</module>
     <module>log4j-mongodb4</module>
+    <module>log4j-mongodb</module>

Review Comment:
   Well, since db depends on db4, the order matters; or will Maven reorder in memory?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Feature/2.x/mongodb next gg (logging-log4j2)

Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #2486:
URL: https://github.com/apache/logging-log4j2/pull/2486#discussion_r1572459594


##########
log4j-mongodb/src/main/java/org/apache/logging/log4j/mongodb/package-info.java:
##########
@@ -15,12 +15,13 @@
  * limitations under the license.
  */
 /**
- * The classes in this package contain the MongoDB provider for the NoSQL Appender.
+ * The classes in this package contain the MongoDB provider for the NoSQL
+ * Appender.
  */
 @Export
 @Open("org.apache.logging.log4j.core")
-@Version("2.20.1")
-package org.apache.logging.log4j.mongodb3;
+@Version("2.21.0")

Review Comment:
   Good catch!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Feature/2.x/mongodb next gg (logging-log4j2)

Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #2486:
URL: https://github.com/apache/logging-log4j2/pull/2486#discussion_r1572434330


##########
log4j-mongodb/src/main/java/org/apache/logging/log4j/mongodb/package-info.java:
##########
@@ -15,12 +15,13 @@
  * limitations under the license.
  */
 /**
- * The classes in this package contain the MongoDB provider for the NoSQL Appender.
+ * The classes in this package contain the MongoDB provider for the NoSQL
+ * Appender.
  */
 @Export
 @Open("org.apache.logging.log4j.core")
-@Version("2.20.1")
-package org.apache.logging.log4j.mongodb3;
+@Version("2.21.0")

Review Comment:
   Let us use `2.24.0`, i.e. the version when the class was introduced. It is then easier to find the last Log4j version with some real changes to the module.



##########
log4j-mongodb/src/test/java/org/apache/logging/log4j/mongodb/MongoDbResolver.java:
##########
@@ -142,11 +144,11 @@ private static final class MongoClientHolder implements CloseableResource, Suppl
         private final MongoClient mongoClient;
 
         public MongoClientHolder(final Mongod mongod, final TestProperties props) {
-            state = mongod.start(Version.Main.V3_6);
+            state = mongod.start(Version.Main.V4_4);

Review Comment:
   ```suggestion
               state = mongod.start(Version.Main.V7_0_8);
   ```
   
   Shouldn't we use the newest server version compatible with the client?



##########
pom.xml:
##########
@@ -257,8 +257,8 @@
     <module>log4j-jul</module>
     <module>log4j-layout-template-json</module>
     <module>log4j-layout-template-json-test</module>
-    <module>log4j-mongodb3</module>
     <module>log4j-mongodb4</module>
+    <module>log4j-mongodb</module>

Review Comment:
   _Nit_: shouldn't we use a lexicographic (alphabetic) order?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org