Alon Bar-Lev has posted comments on this change. Change subject: extensions test tool: logger ......................................................................
Patch Set 23: (6 comments) https://gerrit.ovirt.org/#/c/37886/23/backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/core/ExtensionsToolExecutor.java File backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/core/ExtensionsToolExecutor.java: Line 130: return modules; Line 131: } Line 132: Line 133: private static void setupLogger() { Line 134: OVIRT_LOGGER.setLevel(Level.parse("INFO")); where do you take the log level from environment? Line 135: } Line 136: Line 137: private static void setupLogger(Map<String, Object> args) throws IOException { Line 138: Logger logger = Logger.getLogger(""); https://gerrit.ovirt.org/#/c/37886/23/backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/logger/LoggerServiceImpl.java File backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/logger/LoggerServiceImpl.java: Line 102: } Line 103: throw new ExitException("Parsing error", 1); Line 104: } Line 105: logger.debug( Line 106: "Using action arguments: level={} message={} extension-name={}", please quote the message and extension name, please move extension name to be first. Line 107: actionArgs.get("level"), Line 108: actionArgs.get("message"), Line 109: actionArgs.get("extension-name") Line 110: ); Line 119: System.out.format( Line 120: "usage: %s", Line 121: parser.getUsage() Line 122: .replace("@PROGRAM_NAME@", (String) context.get(PROGRAM_NAME)) Line 123: .replace("@ACTION_LIST@", StringUtils.join(actions.keySet(), ", ")) this is the only reason to introduce commons-lang dependency... not sure it worth it. Line 124: ); Line 125: } Line 126: Line 127: private void logrecord() { Line 161: Base.InvokeKeys.COMMAND, Line 162: Logger.InvokeCommands.CLOSE Line 163: ) Line 164: ); Line 165: logger.info("Log-record action has been invoked"); completed? Line 166: } https://gerrit.ovirt.org/#/c/37886/23/ovirt-engine.spec.in File ovirt-engine.spec.in: Line 479: Line 480: %package extension-tool Line 481: Summary: %{ovirt_product_name_short} Extension Tool Line 482: Group: %{ovirt_product_group} Line 483: Requires: %{name} = %{version}-%{release} this is interesting, we probably need %{name}, explicit dependency will break upgrade. maybe best to remove it entirely... or... maybe we can remove dependency of engine, this will require removing the usage of engine-prolog. hmmm... it is not enough we need the slf4j and other modules. we have egg and chicken because of the engine invalid use of version lock. so I am sorry... we cannot have this as separate package in current shape of engine... please forgive me for trying... embed this in tools subpackage, it will be very difficult to change this later as this subpackage is versionlocked, but I see no choice now. Line 484: Requires: %{name}-lib >= %{version}-%{release} Line 485: Requires: java Line 486: Line 487: %description extension-tool https://gerrit.ovirt.org/#/c/37886/23/packaging/services/ovirt-engine/ovirt-engine.conf.in File packaging/services/ovirt-engine/ovirt-engine.conf.in: Line 21: Line 22: # Line 23: # Default logging level Line 24: # Line 25: OVIRT_LOGGING_LEVEL="INFO" it should be environment not configuration :))) Line 26: Line 27: # Line 28: # The location of the application server used by the engine: Line 29: # -- To view, visit https://gerrit.ovirt.org/37886 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie06113c5d56a49e58d557c851f9ff00b9a9ca409 Gerrit-PatchSet: 23 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ondřej Macháček <machacek.on...@gmail.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Ondra Machacek <omach...@redhat.com> Gerrit-Reviewer: Ondřej Macháček <machacek.on...@gmail.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer <mta...@redhat.com> 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
