Alon Bar-Lev has posted comments on this change. Change subject: extensions test tool: logger ......................................................................
Patch Set 13: (7 comments) ok, general note for the .in files including the config, as we need to use these anyway, we should generate them in Makefile and put them in the classes of the tool module, this will enable us to add the resources after maven executed with proper substitutions of make, and still work in mead environment for now. https://gerrit.ovirt.org/#/c/37886/13/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 69: } catch (Throwable t) { Line 70: String message = t.getMessage() != null ? t.getMessage() : t.getClass().getName(); Line 71: logger.error(message); Line 72: logger.debug(message, t); Line 73: t.printStackTrace(); this is done automatically in debug, no? Line 74: } Line 75: System.exit(exitStatus); Line 76: } Line 77: Line 115: throw new ExitException("Please provide module.", 0); Line 116: } Line 117: Line 118: return new ParametersParser( Line 119: ExtensionsToolExecutor.class.getResourceAsStream("arguments.properties.in") .in should not be in jar, it is template for makefile. Line 120: ).parse(args); Line 121: } Line 122: Line 123: private static void setupLogger(Map<String, Object> args) throws IOException{ https://gerrit.ovirt.org/#/c/37886/13/backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/logger/services/LoggerServiceImpl.java File backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/logger/services/LoggerServiceImpl.java: Line 63: Line 64: @Override Line 65: public void parseArguments(List<String> argsList) throws Exception { Line 66: args = new ParametersParser( Line 67: getClass().getResourceAsStream("arguments.properties.in") .in should not be in jar, it is only template processed by Makefile. Line 68: ).parse(argsList, true); Line 69: } Line 70: Line 71: @Override https://gerrit.ovirt.org/#/c/37886/13/backend/manager/extension-tool/src/main/resources/org/ovirt/engine/exttool/logger/services/arguments.properties.in File backend/manager/extension-tool/src/main/resources/org/ovirt/engine/exttool/logger/services/arguments.properties.in: Line 17: log-record.arg.level.convert = java.util.logging.Level Line 18: log-record.arg.level.default = INFO Line 19: log-record.arg.help.name = help Line 20: log-record.arg.help.help = show help for logger module Line 21: underlinetext = You can test your logger extension new line please. log-record.help.underlinetext ? https://gerrit.ovirt.org/#/c/37886/13/backend/manager/modules/uutils/src/main/java/org/ovirt/engine/core/uutils/cli/parser/ParametersParser.java File backend/manager/modules/uutils/src/main/java/org/ovirt/engine/core/uutils/cli/parser/ParametersParser.java: Line 50: } Line 51: Line 52: public Map<String, Object> parse(String[] args, boolean isModule) throws Exception { Line 53: return parse(new ArrayList<String>(Arrays.asList(args)), isModule); Line 54: } I do not understand why we have a difference between module or not, please explain? I see the comment but cannot understand. Line 55: Line 56: public Map<String, Object> parse(List<String> args, boolean isModule) throws Exception { Line 57: String usage = "core"; Line 58: List<Object> others = new ArrayList<>(); Line 265: } Line 266: Line 267: public static Properties loadProperties(InputStream resource) { Line 268: try ( Line 269: Reader is = new InputStreamReader(resource, "UTF-8"); what did you revert the Charset.forName()? Line 270: ) { Line 271: Properties prop = new Properties(); Line 272: prop.load(is); Line 273: return prop; https://gerrit.ovirt.org/#/c/37886/13/backend/manager/modules/uutils/src/main/resources/org/ovirt/engine/core/uutils/cli/parser/defaults.properties File backend/manager/modules/uutils/src/main/resources/org/ovirt/engine/core/uutils/cli/parser/defaults.properties: Line 4: arg.convert = java.lang.String Line 5: arg.matcher = .* Line 6: arg.metavar = STRING Line 7: arg.multivalue = false Line 8: underlinetext = new line please. please always add category, in this case help. or usage. so we can have multi value in future. -- 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: 13 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
