Alon Bar-Lev has posted comments on this change. Change subject: core: Replace oVirt logger with slf4j in utils ......................................................................
Patch Set 2: (7 comments) http://gerrit.ovirt.org/#/c/34264/2/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ErrorTranslatorImpl.java File backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ErrorTranslatorImpl.java: Line 64: log.warn("Code '{}' appears more than once in string table.", key); Line 65: } Line 66: } Line 67: } catch (RuntimeException e) { Line 68: log.error("File: '{}' could not be loaded: {}", messageSource, e.toString()); e.getMessage() Line 69: log.debug("Exception", e); Line 70: } Line 71: return messages; Line 72: } http://gerrit.ovirt.org/#/c/34264/2/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/OsRepositoryImpl.java File backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/OsRepositoryImpl.java: Line 58: buildIdToUnameLookup(); Line 59: buildBackCompatMapping(); Line 60: validateTree(); Line 61: if (log.isDebugEnabled()) { Line 62: log.debug("Osinfo Repository:\n {}", toString()); please drop the space after the \n Line 63: } Line 64: } Line 65: Line 66: private void validateTree() { Line 79: } Line 80: } Line 81: } catch (BackingStoreException e) { Line 82: log.warn("Failed to validate Os Repository due to {}", e); Line 83: log.debug("Exception", e); no need to print exception in both the {} will not be translated, either change the first to e.getMessage() or drop the debug. Line 84: throw new RuntimeException("Failed to validate Os Repository due to " + e); Line 85: } Line 86: } Line 87: Line 595: StringBuilder sb = new StringBuilder(); Line 596: try { Line 597: walkTree(sb, preferences); Line 598: } catch (BackingStoreException e) { Line 599: log.error(e.getMessage()); please provide a string Line 600: log.debug("Exception", e); Line 601: } Line 602: return sb.toString(); Line 603: } http://gerrit.ovirt.org/#/c/34264/2/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ResourceUtils.java File backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ResourceUtils.java: Line 40: try { Line 41: is.close(); Line 42: } catch (Exception e) { Line 43: String msg = (e.getMessage() != null) ? e.getMessage() : ""; Line 44: log.error("Failed to close input stream: {}", msg); I think you can drop the msg and simply use e.getMessage() Line 45: log.debug("Exception", e); Line 46: } Line 47: } Line 48: } http://gerrit.ovirt.org/#/c/34264/2/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/threadpool/ThreadPoolUtil.java File backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/threadpool/ThreadPoolUtil.java: Line 48: if (!threadName.startsWith("org.ovirt.thread.")) { Line 49: t.setName("org.ovirt.thread." + threadName); Line 50: } Line 51: if (log.isDebugEnabled()) { Line 52: log.debug(String.format("About to run task '%s' from {}", r.getClass().getName()), new Exception()); I do not like this... but... I do understand. anyway, please drop the {} it won't get translated. Line 53: } Line 54: Line 55: if (getQueue().size() > 5) { Line 56: log.warn("Executing a command '{}', but note that there are {} tasks in the queue.", http://gerrit.ovirt.org/#/c/34264/2/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/RandomUtilsSeedingRule.java File backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/RandomUtilsSeedingRule.java: Line 31: RANDOM_SEED_PROPERTY); Line 32: seed = System.currentTimeMillis(); Line 33: } Line 34: RandomUtils.instance().setSeed(seed); Line 35: log.info("Running test with random seed: {}", RandomUtils.instance().getSeed()); this was a bug... it should be seed and log before the set? Line 36: } -- To view, visit http://gerrit.ovirt.org/34264 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0b99ca3947a3ad778601da27decdbad751cfa19c 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: Martin Peřina <mper...@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
