Ondřej Macháček has posted comments on this change. Change subject: extensions test tool: logger ......................................................................
Patch Set 18: (5 comments) https://gerrit.ovirt.org/#/c/37886/18/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 70: if (moduleService == null) { Line 71: logger.error(String.format("No such '%1$s' module exists.", module)); Line 72: throw new ExitException(1); Line 73: } Line 74: loadExtensions(moduleService, argMap); > loadExtensions after module parse argument? so we avoid doing anything befo ok :) Line 75: moduleService.parseArguments(moduleOthers); Line 76: moduleService.run(); Line 77: } catch(ExitException e) { Line 78: logger.debug(e.getMessage(), e); https://gerrit.ovirt.org/#/c/37886/18/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 63: public void parseArguments(List<String> moduleArgs) throws Exception { Line 64: moduleArgs.remove(0); Line 65: ParametersParser parser = new ParametersParser( Line 66: getClass().getResourceAsStream("arguments.properties"), Line 67: getName() > I think that the prefix of the module core arguments should be the same for I will change that. Line 68: ); Line 69: args.putAll(parser.parse(moduleArgs)); Line 70: if(args.containsKey("help")) { Line 71: printUsage(parser); Line 65: ParametersParser parser = new ParametersParser( Line 66: getClass().getResourceAsStream("arguments.properties"), Line 67: getName() Line 68: ); Line 69: args.putAll(parser.parse(moduleArgs)); > why do you need to copy args? what am I missing? logically you should be ab Well, I do not need this in this module, as the module args is just --help. But in other modules, there could be some other args, which could we need in specific runnable method. so I've added it to global args of class. Line 70: if(args.containsKey("help")) { Line 71: printUsage(parser); Line 72: throw new ExitException(); Line 73: } Line 71: printUsage(parser); Line 72: throw new ExitException(); Line 73: } Line 74: Line 75: List<String> actionOthers = (List<String>)args.get("__other__"); > I guess "__other__" can be a static constaint within parser :) ok:) Line 76: String action = actionOthers.remove(0); Line 77: actionMethod = actions.get(action); Line 78: if(actionMethod == null) { Line 79: throw new IllegalArgumentException( https://gerrit.ovirt.org/#/c/37886/18/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 122: ((List<Object>)o).add(convertedValue); Line 123: } Line 124: } Line 125: } Line 126: mandatory.removeAll(argMap.keySet()); > better to copy so that you can call parse() twice? ok Line 127: if(!mandatory.isEmpty() && !argMap.containsKey("help")) { Line 128: throw new ExitException( Line 129: String.format("Argument(s) '%1$s' required", StringUtils.join(mandatory, ", ")), Line 130: 1 -- 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: 18 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
