Alon Bar-Lev has posted comments on this change. Change subject: core: Add common methods for runtime log4j setup ......................................................................
Patch Set 1: (3 comments) http://gerrit.ovirt.org/#/c/26588/1/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/log/Log4jUtils.java File backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/log/Log4jUtils.java: Line 34: if (f.exists()) { Line 35: url = f.toURI().toURL(); Line 36: } else { Line 37: url = new URL(log4jConfig); Line 38: } > It's because of DomConfigurator(URL) method: this is advanced user feature.... advance users can specify URLs. but... checking if file exist to determine if this is url or not is not valid... you should check for [^:]+://.* pattern or something. Line 39: } Line 40: LogManager.resetConfiguration(); Line 41: DOMConfigurator.configure(url); Line 42: } catch (IOException | FactoryConfigurationError ex) { Line 67: FileAppender fa = null; Line 68: try { Line 69: fa = new FileAppender(new PatternLayout("%d %-5p [%c] %m%n"), fileName, true); Line 70: fa.setThreshold(level); Line 71: } catch (SecurityException | IOException ex) { > Well, the exception can be thrown only in this call. Also if entire functio ok, although I do not see how other cases you have these exceptions, nor do I understand why it is important why exactly we cannot access the log file. Line 72: throw new IllegalArgumentException( Line 73: String.format("Error accessing log file '%s': '%s'", fileName, ex.getMessage()), Line 74: ex); Line 75: } Line 77: Logger rootLogger = Logger.getRootLogger(); Line 78: if (rootLogger.getLevel() == Level.OFF) { Line 79: // by default it's set to OFF, so we need to enabled it before adding appender Line 80: rootLogger.setLevel(Level.ALL); Line 81: } > By default I had to set root logger level to OFF, otherwise "log4j No appen which should be solved by adding null appender or any other solution that does not touch the root. Line 82: rootLogger.addAppender(fa); Line 83: } Line 84: } -- To view, visit http://gerrit.ovirt.org/26588 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic00ea0af643e8c978fd67eb8c56e2329d43e776d Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
