Alon Bar-Lev has posted comments on this change.

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


Patch Set 3:

(3 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 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);
> This logging configuration file is entered by used in --log-config tool com
I thought of having default embedded property file or user provided file, and 
not require any code to load this... much simpler especially for debugging.

If you like, we can provide alternate logging property files via environment 
and not cmdline, so it will be easier for us to handle at wrapper level.
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
> I just verified a few minutes ago, if I change to .level=OFF and in command
but why?

something is wrong. as you should be able to override the level, or provide 
somewhere default sane value.

maybe this level is for the appender? and we need to set a level for the logger?
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
> In JavaLoggingUtils.addFileHandler(), this formatter is used only for log f
this should be here not at code... this is why there are property files, so we 
can reduce the code.

same should have done for the arguments... if you remember... but I gave up 
then :(


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