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/01/30 00:25:46 UTC

[GitHub] [logging-log4j2] ppkarwasz opened a new pull request #737: Add missing properties and fixes default values

ppkarwasz opened a new pull request #737:
URL: https://github.com/apache/logging-log4j2/pull/737


   The properties of the generated appenders are not always the same as in Log4j 1.x. This PR:
   
    - adds default values to all Log4j 1.x configuration builders,
    - adds a couple of properties that were missing in the builders,
    - expands the test suite to include the case of all properties unspecified and all properties set to a value different from the default.
   
   Problems with default values cause issues as in [this SO question](https://stackoverflow.com/q/70895631/11748454).


-- 
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] vy commented on pull request #737: Adds support for missing Log4j 1.x properties and fixes default values

Posted by GitBox <gi...@apache.org>.
vy commented on pull request #737:
URL: https://github.com/apache/logging-log4j2/pull/737#issuecomment-1028089316


   @garydgregory, would you be interested in taking this on as a part of your ongoing `log4j-1.2-bridge` crusade?


-- 
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 a change in pull request #737: Adds support for missing Log4j 1.x properties and fixes default values

Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #737:
URL: https://github.com/apache/logging-log4j2/pull/737#discussion_r799819579



##########
File path: log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/ConsoleAppenderBuilder.java
##########
@@ -107,6 +110,24 @@ public Appender parseAppender(final Element appenderElement, final XmlConfigurat
                             }
                             break;
                         }
+                        case FOLLOW_PARAM: {
+                            String value = getValueAttribute(currentElement);
+                            if (value == null) {
+                                LOGGER.warn("No value supplied for Follow parameter. Using default of {}", false);
+                            } else {
+                                follow.set(Boolean.valueOf(value));
+                            }
+                            break;
+                        }
+                        case IMMEDIATE_FLUSH_PARAM: {
+                            String value = getValueAttribute(currentElement);
+                            if (value == null) {
+                                LOGGER.warn("No value supplied for ImmediateFlush parameter. Using default of {}", true);
+                            } else if (!Boolean.getBoolean(name)) {
+                                LOGGER.warn("The value {} for ImmediateFlush parameter is not supported.", false);
+                            }

Review comment:
       FYI : This seems incomplete, where is the value used?




-- 
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 merged pull request #737: Adds support for missing Log4j 1.x properties and fixes default values

Posted by GitBox <gi...@apache.org>.
garydgregory merged pull request #737:
URL: https://github.com/apache/logging-log4j2/pull/737


   


-- 
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 #737: Adds support for missing Log4j 1.x properties and fixes default values

Posted by GitBox <gi...@apache.org>.
ppkarwasz commented on a change in pull request #737:
URL: https://github.com/apache/logging-log4j2/pull/737#discussion_r800038422



##########
File path: log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/ConsoleAppenderBuilder.java
##########
@@ -107,6 +110,24 @@ public Appender parseAppender(final Element appenderElement, final XmlConfigurat
                             }
                             break;
                         }
+                        case FOLLOW_PARAM: {
+                            String value = getValueAttribute(currentElement);
+                            if (value == null) {
+                                LOGGER.warn("No value supplied for Follow parameter. Using default of {}", false);
+                            } else {
+                                follow.set(Boolean.valueOf(value));
+                            }
+                            break;
+                        }
+                        case IMMEDIATE_FLUSH_PARAM: {
+                            String value = getValueAttribute(currentElement);
+                            if (value == null) {
+                                LOGGER.warn("No value supplied for ImmediateFlush parameter. Using default of {}", true);
+                            } else if (!Boolean.getBoolean(name)) {
+                                LOGGER.warn("The value {} for ImmediateFlush parameter is not supported.", false);
+                            }

Review comment:
       Since the Log4j2 `ConsoleAppender` does not use the `immediateFlush` parameter (cf. [`ConsoleAppender.Builder#build`](https://github.com/apache/logging-log4j2/blob/0de646bde36d2c426988970314428d68b8858a5a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/ConsoleAppender.java#L218)), I only issue a warning that the value supplied by the user will be ignored.
   Alternatively the Log4j2 `ConsoleAppender` could issue a warning if `immediateValue`, `bufferedIO` or `bufferSize` differ from the default values.




-- 
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 #737: Adds support for missing Log4j 1.x properties and fixes default values

Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #737:
URL: https://github.com/apache/logging-log4j2/pull/737#issuecomment-1028104019


   @vy I might be able to take a look this weekend. 


-- 
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