You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by GitBox <gi...@apache.org> on 2022/02/08 22:18:34 UTC
[GitHub] [logging-log4j2] ppkarwasz opened a new pull request #745: [LOG4J2-3362] Adds a SmtpManager compatible with Jakarta EE 9
ppkarwasz opened a new pull request #745:
URL: https://github.com/apache/logging-log4j2/pull/745
Since `javax.mail` and `jakarta.mail` use different package names, as single `SmtpAppender` can use both through different managers.
Unfortunately the implementations `com.sun.mail:javax.mail` and `com.sun.mail:jakarta.mail` use the same class names, so it is not
possible to test both implementations at the same time. A separate `test` execution was configured for the tests using `jakarta.*` classes.
--
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
[GitHub] [logging-log4j2] garydgregory commented on pull request #745: [LOG4J2-3362] Adds a SmtpManager compatible with Jakarta EE 9
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #745:
URL: https://github.com/apache/logging-log4j2/pull/745#issuecomment-1033126921
IMO, anything new that depends on other dependencies should be in a new Maven module.
--
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
[GitHub] [logging-log4j2] ppkarwasz commented on pull request #745: [LOG4J2-3362] Adds a SmtpManager compatible with Jakarta EE 9
Posted by GitBox <gi...@apache.org>.
ppkarwasz commented on pull request #745:
URL: https://github.com/apache/logging-log4j2/pull/745#issuecomment-1048291947
@garydgregory: I moved the `SmtpManager` to a new `log4j-jakarta-smtp` module.
The choice between the `javax.*` and `jakarta.*` version is automatic, based on the classpath, but can be forced with a new `useJakarta` attribute.
--
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
[GitHub] [logging-log4j2] ppkarwasz commented on pull request #745: [LOG4J2-3362] Adds a SmtpManager compatible with Jakarta EE 9
Posted by GitBox <gi...@apache.org>.
ppkarwasz commented on pull request #745:
URL: https://github.com/apache/logging-log4j2/pull/745#issuecomment-1082395718
@astappiev,
As far as I am concerned, this could end up in the next release. You can check if it works for you.
--
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
[GitHub] [logging-log4j2] ppkarwasz edited a comment on pull request #745: [LOG4J2-3362] Adds a SmtpManager compatible with Jakarta EE 9
Posted by GitBox <gi...@apache.org>.
ppkarwasz edited a comment on pull request #745:
URL: https://github.com/apache/logging-log4j2/pull/745#issuecomment-1033157134
@garydgregory: I think of this as a nice application of the separation between appenders and managers, which mitigates the problems of the jakartification.
However I see your point: the `SmtpAppender` and `JmsAppender` probably should be moved out of `log4j-core` and a jakartified version should be provided separately.
--
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
[GitHub] [logging-log4j2] ppkarwasz edited a comment on pull request #745: [LOG4J2-3362] Adds a SmtpManager compatible with Jakarta EE 9
Posted by GitBox <gi...@apache.org>.
ppkarwasz edited a comment on pull request #745:
URL: https://github.com/apache/logging-log4j2/pull/745#issuecomment-1034024340
@jvz: is there any sense in using `javax.*` in the 3.0 release? Spring Boot 6.0 with a baseline of Java 17 and Jakarta EE 9 is due at the end of this year. How many people are eager to upgrade to Log4j 3.x, but keep an old Spring version?
--
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
[GitHub] [logging-log4j2] ppkarwasz commented on pull request #745: [LOG4J2-3362] Adds a SmtpManager compatible with Jakarta EE 9
Posted by GitBox <gi...@apache.org>.
ppkarwasz commented on pull request #745:
URL: https://github.com/apache/logging-log4j2/pull/745#issuecomment-1033157134
@garydgregory: I think of this as a nice application of the separation between appenders and managers, which mitigates the problems of the jakartification.
However I see you point: the `SmtpAppender` and `JmsAppender` probably should be moved out of `log4j-core` and a jakartified version should be provided separately.
--
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
[GitHub] [logging-log4j2] ppkarwasz commented on a change in pull request #745: [LOG4J2-3362] Adds a SmtpManager compatible with Jakarta EE 9
Posted by GitBox <gi...@apache.org>.
ppkarwasz commented on a change in pull request #745:
URL: https://github.com/apache/logging-log4j2/pull/745#discussion_r802111545
##########
File path: log4j-core/src/test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java
##########
@@ -39,14 +39,15 @@
public class SmtpManagerTest {
@Test
- void testCreateManagerName() {
- String managerName = SmtpManager.createManagerName("to", "cc", null, "from", null, "LOG4J2-3107",
- "proto", "smtp.log4j.com", 4711, "username", false, "filter");
+ public void testCreateManagerName() {
Review comment:
These tests were disabled (non-public).
--
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
[GitHub] [logging-log4j2] andrei-ivanov commented on a change in pull request #745: [LOG4J2-3362] Adds a SmtpManager compatible with Jakarta EE 9
Posted by GitBox <gi...@apache.org>.
andrei-ivanov commented on a change in pull request #745:
URL: https://github.com/apache/logging-log4j2/pull/745#discussion_r820284822
##########
File path: log4j-core/src/test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java
##########
@@ -39,14 +39,15 @@
public class SmtpManagerTest {
@Test
- void testCreateManagerName() {
- String managerName = SmtpManager.createManagerName("to", "cc", null, "from", null, "LOG4J2-3107",
- "proto", "smtp.log4j.com", 4711, "username", false, "filter");
+ public void testCreateManagerName() {
Review comment:
JUnit Jupiter doesn't need tests to be public, I think it's actually recommended to use the default visibility
--
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
[GitHub] [logging-log4j2] jvz commented on pull request #745: [LOG4J2-3362] Adds a SmtpManager compatible with Jakarta EE 9
Posted by GitBox <gi...@apache.org>.
jvz commented on pull request #745:
URL: https://github.com/apache/logging-log4j2/pull/745#issuecomment-1034062599
I think as long as the maintenance burden is low enough, we can keep support for the old APIs indefinitely. Our Java baseline there is 11, so not as aggressive as Spring's baseline. That is a good point, though.
--
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
[GitHub] [logging-log4j2] ppkarwasz commented on a change in pull request #745: [LOG4J2-3362] Adds a SmtpManager compatible with Jakarta EE 9
Posted by GitBox <gi...@apache.org>.
ppkarwasz commented on a change in pull request #745:
URL: https://github.com/apache/logging-log4j2/pull/745#discussion_r802110065
##########
File path: pom.xml
##########
@@ -687,10 +691,43 @@
<optional>true</optional>
</dependency>
<!-- Jackson 2 end -->
+ <dependency>
+ <groupId>javax.mail</groupId>
+ <artifactId>javax.mail-api</artifactId>
+ <version>${javax.mail.version}</version>
+ <optional>true</optional>
+ </dependency>
+ <dependency>
+ <groupId>javax.activation</groupId>
+ <artifactId>javax.activation-api</artifactId>
+ <version>${javax.activation.version}</version>
+ <optional>true</optional>
+ </dependency>
<dependency>
<groupId>com.sun.mail</groupId>
<artifactId>javax.mail</artifactId>
- <version>1.6.2</version>
+ <version>${javax.mail.version}</version>
+ <scope>runtime</scope>
+ <optional>true</optional>
+ </dependency>
+ <dependency>
+ <groupId>jakarta.mail</groupId>
+ <artifactId>jakarta.mail-api</artifactId>
+ <version>${jakarta.mail.version}</version>
+ <optional>true</optional>
+ </dependency>
+ <dependency>
+ <groupId>jakarta.activation</groupId>
+ <artifactId>jakarta.activation-api</artifactId>
+ <version>${jakarta.activation.version}</version>
+ <optional>true</optional>
+ </dependency>
+ <dependency>
+ <groupId>com.sun.mail</groupId>
+ <artifactId>jakarta.mail</artifactId>
+ <version>${jakarta.mail.version}</version>
+ <scope>runtime</scope>
+ <optional>true</optional>
Review comment:
Same philosophy as `logback-classic`: use APIs as (optional) compile dependencies, while the implementation is an (optional) runtime dependency.
--
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
[GitHub] [logging-log4j2] jvz commented on pull request #745: [LOG4J2-3362] Adds a SmtpManager compatible with Jakarta EE 9
Posted by GitBox <gi...@apache.org>.
jvz commented on pull request #745:
URL: https://github.com/apache/logging-log4j2/pull/745#issuecomment-1033294601
> However I see you point: the `SmtpAppender` and `JmsAppender` probably should be moved out of `log4j-core` and a jakartified version should be provided separately.
Already prepared as separate modules in the 3.0.0 branch: https://github.com/apache/logging-log4j2/tree/master
However, those two modules (log4j-jms and log4j-smtp) would still need corresponding jakarta variants.
--
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
[GitHub] [logging-log4j2] ppkarwasz commented on pull request #745: [LOG4J2-3362] Adds a SmtpManager compatible with Jakarta EE 9
Posted by GitBox <gi...@apache.org>.
ppkarwasz commented on pull request #745:
URL: https://github.com/apache/logging-log4j2/pull/745#issuecomment-1034024340
@jvz: is there any sense in using `javax.*` in the 3.0 release? Spring Boot 6.0 with a baseline of Java 17 and Jakarta EE 9 is due at the end of this year.
--
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
[GitHub] [logging-log4j2] astappiev commented on pull request #745: [LOG4J2-3362] Adds a SmtpManager compatible with Jakarta EE 9
Posted by GitBox <gi...@apache.org>.
astappiev commented on pull request #745:
URL: https://github.com/apache/logging-log4j2/pull/745#issuecomment-1081915601
Any progress on this or release date?
--
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