Yair Zaslavsky has posted comments on this change. Change subject: aaa: Extensions tester tool ......................................................................
Patch Set 7: (11 comments) http://gerrit.ovirt.org/#/c/27814/7/backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/ExtensionsTool.java File backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/ExtensionsTool.java: Line 76: } else if (args.contains(ExtensionsToolArguments.ARG_PASSWORD_FILE)) { Line 77: try (BufferedReader reader = new BufferedReader(new InputStreamReader( new FileInputStream(args.get(ExtensionsToolArguments.ARG_PASSWORD_FILE))))) { Line 78: password = reader.readLine(); Line 79: } Line 80: } else { > else should be exception... check pass: explicitly. Done Line 81: System.out.print("Please enter password: "); Line 82: password = new String(System.console().readPassword()); Line 83: } Line 84: http://gerrit.ovirt.org/#/c/27814/7/backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/ExtensionsToolExecutor.java File backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/ExtensionsToolExecutor.java: Line 24: System.out.println(t.getMessage()); Line 25: System.exit(1); Line 26: } Line 27: Line 28: if (testerArgs.contains(ACTION_HELP)) { > not sure I understand how you check help at the end... i will fix (copy form mng domains code)> Line 29: testerArgs.printHelp(); Line 30: System.exit(0); Line 31: } Line 32: } http://gerrit.ovirt.org/#/c/27814/7/backend/manager/extension-tool/src/main/modules/module.xml File backend/manager/extension-tool/src/main/modules/module.xml: Line 1: <?xml version="1.0" encoding="UTF-8"?> Line 2: Line 3: <module xmlns="urn:jboss:module:1.1" name="org.ovirt.engine.core.tools"> this file should not be a part of the patch. Line 4: Line 5: <resources> Line 6: <resource-root path="tools.jar"/> Line 7: </resources> http://gerrit.ovirt.org/#/c/27814/7/backend/manager/extension-tool/src/main/modules/org/ovirt/engine/core/extension-tool/main/module.xml File backend/manager/extension-tool/src/main/modules/org/ovirt/engine/core/extension-tool/main/module.xml: Line 9: <dependencies> Line 10: <module name="org.ovirt.engine.api.ovirt-engine-extensions-api"/> Line 11: <module name="org.ovirt.engine.core.extensions-manager"/> Line 12: <module name="org.ovirt.engine.core.utils"/> Line 13: <module name="sun.jdk"/> > you do not need the above two Done Line 14: </dependencies> http://gerrit.ovirt.org/#/c/27814/7/backend/manager/extension-tool/src/main/resources/extensions-tool/help.properties File backend/manager/extension-tool/src/main/resources/extensions-tool/help.properties: Line 1: help.usage=usage: extensions-tool <action> [<args>] > please do not add man pages we should be able to generate these from the us and what should --help present? Line 2: Line 3: help.actions=\ Line 4: Available actions:\ Line 5: \n\tauth-sequence runs an authentication sequence (authenticate, fetch principal, logout)\ http://gerrit.ovirt.org/#/c/27814/7/backend/manager/extension-tool/src/main/resources/extensions-tool/log4j.xml File backend/manager/extension-tool/src/main/resources/extensions-tool/log4j.xml: Line 12: command line argument is specified. Line 13: --> Line 14: <appender-ref ref="null-appender" /> Line 15: </root> Line 16: </log4j:configuration> > you are not using log4j no need to add Done http://gerrit.ovirt.org/#/c/27814/7/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/AuthzUtils.java File backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/AuthzUtils.java: > please do not use this in tool, and if change is required, please move it t Done Line 1: package org.ovirt.engine.core.aaa; Line 2: Line 3: import java.util.ArrayList; Line 4: import java.util.Arrays; http://gerrit.ovirt.org/#/c/27814/7/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/SearchQueryParsingUtils.java File backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/SearchQueryParsingUtils.java: > please do not use this in tool, and if change is required, please move it t Done Line 1: package org.ovirt.engine.core.aaa; Line 2: Line 3: import java.util.ArrayList; Line 4: import java.util.Arrays; http://gerrit.ovirt.org/#/c/27814/7/ovirt-engine.spec.in File ovirt-engine.spec.in: Line 594: %{engine_jboss_modules}/org/ovirt/engine/api/ovirt-engine-extensions-api/main/ovirt-engine-extensions-api.jar Line 595: %{engine_jboss_modules}/org/ovirt/engine/core/aaa/main/aaa.jar Line 596: %{engine_jboss_modules}/org/ovirt/engine/core/common/main/common.jar Line 597: %{engine_jboss_modules}/org/ovirt/engine/core/compat/main/compat.jar Line 598: %{engine_jboss_modules}/org/ovirt/engine/core/extension-tool/main/extension-tool.jar > please sort() Done Line 599: %{engine_jboss_modules}/org/ovirt/engine/core/dal/main/dal.jar Line 600: %{engine_jboss_modules}/org/ovirt/engine/core/extensions-manager/main/extensions-manager.jar Line 601: %{engine_jboss_modules}/org/ovirt/engine/core/searchbackend/main/searchbackend.jar Line 602: %{engine_jboss_modules}/org/ovirt/engine/core/tools/main/tools.jar http://gerrit.ovirt.org/#/c/27814/7/packaging/bin/ovirt-engine-extensions-tool.sh File packaging/bin/ovirt-engine-extensions-tool.sh: Line 19: # Line 20: Line 21: exec "${JAVA_HOME}/bin/java" \ Line 22: -Djboss.modules.write-indexes=false \ Line 23: -Djava.security.auth.login.config="${ENGINE_USR}/conf/jaas.conf" > kerneros? where? Line 24: -jar "${JBOSS_HOME}/jboss-modules.jar" \ Line 25: -dependencies org.ovirt.engine.core.extension-tool \ Line 26: -class org.ovirt.engine.exttool.ExtensionsToolExecutor \ Line 27: -cp "${ENGINE_USR}/conf/commons-logging.properties" Line 23: -Djava.security.auth.login.config="${ENGINE_USR}/conf/jaas.conf" Line 24: -jar "${JBOSS_HOME}/jboss-modules.jar" \ Line 25: -dependencies org.ovirt.engine.core.extension-tool \ Line 26: -class org.ovirt.engine.exttool.ExtensionsToolExecutor \ Line 27: -cp "${ENGINE_USR}/conf/commons-logging.properties" > set using -Djava.util.logging.config.file ? also this should be specific to ok, please advise on the path for these two - is ${ENGINE_USR}/conf/extensions-tool/ ok? -- To view, visit http://gerrit.ovirt.org/27814 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7ea2f9c62ced5bdd3801c9f6d8087a35e3c21886 Gerrit-PatchSet: 7 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@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
