Alon Bar-Lev has posted comments on this change.

Change subject: tools: Replaces log4j with JUL as backend for manage-domains
......................................................................


Patch Set 3:

(4 comments)

http://gerrit.ovirt.org/#/c/32853/3/backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/tools/ManageDomainsExecutor.java
File 
backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/tools/ManageDomainsExecutor.java:

Line 16:  */
Line 17: public class ManageDomainsExecutor {
Line 18:     public static void setupDefaultLogging() {
Line 19:         try (InputStream is = 
ManageDomainsExecutor.class.getResourceAsStream(
Line 20:                 "/engine-manage-domains/logging.properties")) {
> It's the same concept as we currently use for loading log4j.xml. Is there a
so you can use relative loading? nicer... more flexible?
Line 21:             JavaLoggingUtils.setupLogging(is);
Line 22:         } catch (IOException ex) {
Line 23:             throw new IllegalArgumentException(
Line 24:                     String.format("Error loading default loggging 
configuration from: %s", ex.getMessage()),


Line 28: 
Line 29:     public static void setupCustomLogging(String configFile, String 
logFile, String logLevel) {
Line 30:         if (configFile != null) {
Line 31:             try (InputStream is = new FileInputStream(configFile)) {
Line 32:                 JavaLoggingUtils.setupLogging(is);
> Sorry, but I don't understand what do you mean.
you can pass this as -Dxxxxx into the java execution?
Line 33:             } catch (IOException ex) {
Line 34:                 throw new IllegalArgumentException(
Line 35:                         String.format("Error loading loggging 
configuration from '%s': %s", configFile, ex.getMessage()),
Line 36:                         ex);


http://gerrit.ovirt.org/#/c/32853/3/backend/manager/modules/builtin-extensions/src/main/resources/engine-manage-domains/logging.properties
File 
backend/manager/modules/builtin-extensions/src/main/resources/engine-manage-domains/logging.properties:

Line 6: # To change logging properties for engine-manage-domains user command 
line
Line 7: # options --log-file, --log-level and --log-config
Line 8: 
Line 9: # Set root level to ALL, so it can be changed per handler using command 
line parameter --log-level
Line 10: .level=ALL
> It has to be set to ALL, if it's set to OFF, then nothing is logged even wh
Hmmm... I thought we changing the level of this when we do --log-level, aren't 
we?
Line 11: 
Line 12: # Default format of log line is: DATE TIME LEVEL [SOURCE] Message 
Stacktrace


Line 9: # Set root level to ALL, so it can be changed per handler using command 
line parameter --log-level
Line 10: .level=ALL
Line 11: 
Line 12: # Default format of log line is: DATE TIME LEVEL [SOURCE] Message 
Stacktrace
Line 13: java.util.logging.SimpleFormatter.format=%1$tY-%1$tm-%1$td 
%1$tH:%1$tM:%1$tS,%1$tL %4$-5s [%2$s] %5$s%6$s%n
> The syntax is the same as in SimpleFormatter, JavaLoggingFormatter just con
I do not follow... where do you load the JavaLoggingFormatter? it should be 
here, no?


-- 
To view, visit http://gerrit.ovirt.org/32853
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I16afc336c26615978392f7d4ff911bf646c42df6
Gerrit-PatchSet: 3
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: Martin Peřina <mper...@redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@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

Reply via email to