vy commented on code in PR #4199:
URL: https://github.com/apache/logging-log4j2/pull/4199#discussion_r3699998620
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationFactory.java:
##########
@@ -616,12 +628,6 @@ private Configuration getConfiguration(
final ConfigurationSource source =
ConfigurationSource.fromResource(configName, loader);
if (source != null) {
- if (!factory.isActive()) {
Review Comment:
AFAIU, what without this fix, when someone has a project with `log4j.json`,
but without Jackson in the classpath, they were getting an ERROR-level log
message stating that `log4j.json` could not be loaded due to a missing
dependency. With this change, `log4j.json` will not be loaded *and* user will
have no indication of this happened and why. I think this is not desirable from
a user's pov. Shall we change the approach to the following:
- Correctly implement `isActive()` everywhere
- Make `getSupportedTypes()` work, always, regardless of the `isActive()`
state
- Make `getConfiguration()` return null if inactive — this matches the
existing behavior in `CF`
- Use `isActive` correctly & wisely in `CF` such that, if a `CF` is
inactive, don't use it, but still warn the user if an inactive `CF` matches an
existing configuration file
@ramanathan1504, @DragonFSKY, WDYT?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
