ramanathan1504 commented on code in PR #4070:
URL: https://github.com/apache/logging-log4j2/pull/4070#discussion_r3337553136
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/impl/Log4jContextFactory.java:
##########
@@ -300,10 +300,16 @@ public LoggerContext getContext(
final boolean currentContext,
final List<URI> configLocations,
final String name) {
- final LoggerContext ctx =
- selector.getContext(fqcn, loader, currentContext, null /*this
probably needs to change*/);
- if (externalContext != null && ctx.getExternalContext() == null) {
- ctx.setExternalContext(externalContext);
+ final LoggerContext ctx;
+ if (externalContext instanceof Map.Entry) {
Review Comment:
@rgoers
Agreed, but there's a catch:
The externalContext parameter is in the public API. If we stop auto-setting
it, we break existing callers who expect ctx.getExternalContext() to work.
log4j-jakarta-web already uses the right pattern (Map.Entry with distinct
keys), but other integrations might not.
How do you want to handle backward compatibility?
•Breaking change + migration docs?
•Deprecation path?
•Document-only fix?
Happy to implement whichever you prefer.
--
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]
