Alon Bar-Lev has posted comments on this change.

Change subject: core: Add common methods for runtime log4j setup
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.ovirt.org/#/c/26588/2/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 59:             FileAppender fa = null;
Line 60:             try {
Line 61:                 fa = new FileAppender(new PatternLayout("%d %-5p [%c] 
%m%n"), fileName, true);
Line 62:                 fa.setThreshold(level);
Line 63:             } catch (SecurityException | IOException ex) {
> once again, the difference between:
can we please resume to this discussion?

here is the full function as I think should be:

 try {
     Level level = Level.INFO;
     if (levelName != null) {
         level = Level.toLevel(levelName, null);
         if (level == null) {
             throw new IllegalArgumentException(String.format("Invalid log 
level value: '%s'"
, levelName));
         }
         FileAppender fa = new FileAppender(new PatternLayout("%d %-5p [%c] 
%m%n"), fileName, true);
         fa.setThreshold(level);
         Logger.getRootLogger().addAppender(fa);
 } catch (Excpetion e) {
     throw new IllegalArgumentException(
          String.format("Cannot add log appender '%s': %s", fileName, e),
          e
     );
 }

please tell me what do I miss.

even if you understood that level name parsing should be in separate function, 
this function of parsing level name into level should have been in this class.
Line 64:                 throw new IllegalArgumentException(
Line 65:                         String.format("Error accessing log file '%s': 
'%s'", fileName, ex.getMessage()),
Line 66:                         ex);
Line 67:             }


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

Reply via email to