Ondřej Macháček has posted comments on this change. Change subject: manage domains: convert to new parameters parser ......................................................................
Patch Set 2: (6 comments) https://gerrit.ovirt.org/#/c/39972/2/backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/tools/ManageDomains.java File backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/tools/ManageDomains.java: Line 643: if (args.containsKey(ARG_LDAP_SERVERS)) { Line 644: setLdapServersPerDomain(domainName, ldapServersEntry, StringUtils.join(ldapServers, ",")); Line 645: } Line 646: LdapProviderType ldapProviderType = null; Line 647: if (args.containsKey(ARG_PROVIDER)) { > isn't provider mandatory? It is, but don't know why for edit_domain. Line 648: ldapProviderType = LdapProviderType.valueOfIgnoreCase( Line 649: (String)args.get(ManageDomainsArguments.ARG_PROVIDER) Line 650: ); Line 651: } Line 848: passwords.getValueForDomain(domain), Line 849: userGuid, LdapProviderType.valueOf(ldapProviderType.getValueForDomain(domain)), domainLdapServers); Line 850: if (!result.isSuccessful()) { Line 851: if (isValidate || ((domainName != null) && !domain.equals(domainName))) { Line 852: if (args.containsKey(ARG_REPORT)) { > this should be boolean with default false I guess, so check its value and n ok Line 853: System.out.println("WARNING, domain: " + domain + " may not be functional: " Line 854: + result.getDetailedMessage()); Line 855: } else { Line 856: throw result; Line 981: Line 982: configurationProvider.setConfigValue(ConfigValues.LDAPProviderTypes, Line 983: ldapProviderTypeEntry); Line 984: Line 985: if (args.containsKey(ARG_CHANGE_PASSWORD_MSG)) { > this is strange... if argument is provided we put a different value(?) It asks for it interactivelly. in getChangePasswordMsg(). Line 986: configurationProvider.setConfigValue(ConfigValues.ChangePasswordMsg, changePasswordUrlEntry); Line 987: } Line 988: } Line 989: Line 999: } Line 1000: Line 1001: //Prompt warning about last domain only if not "force delete", as using Line 1002: //the force delete option should remove with no confirmation/warning Line 1003: if (domainNameEntry.getDomainNames().size() == 1 && !args.containsKey(ARG_FORCE)) { > the force will be default False, so you can just get it. ok Line 1004: System.out.println(String.format(WARNING_ABOUT_TO_DELETE_LAST_DOMAIN, domainName)); Line 1005: } Line 1006: Line 1007: if(!args.containsKey(ARG_FORCE) && !confirmDeleteDomain(domainName)) { https://gerrit.ovirt.org/#/c/39972/2/backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/tools/ManageDomainsArguments.java File backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/tools/ManageDomainsArguments.java: Line 145 Line 146 Line 147 Line 148 Line 149 > user? oh,true. https://gerrit.ovirt.org/#/c/39972/2/packaging/bin/engine-manage-domains.sh File packaging/bin/engine-manage-domains.sh: Line 23: Line 24: exec "${JAVA_HOME}/bin/java" \ Line 25: -Djava.util.logging.config.file="${OVIRT_LOGGING_PROPERTIES}" \ Line 26: -Djboss.modules.write-indexes=false \ Line 27: -Dorg.ovirt.engine.exttool.core.programName=${0} \ > we should also quote this. ok Line 28: -Dorg.ovirt.engine.exttool.core.packageName="${PACKAGE_NAME}" \ Line 29: -Dorg.ovirt.engine.exttool.core.packageVersion="${PACKAGE_VERSION}" \ Line 30: -Dorg.ovirt.engine.exttool.core.packageDisplayName="${PACKAGE_DISPLAY_NAME}" \ Line 31: -Dorg.ovirt.engine.exttool.core.engineEtc="${ENGINE_ETC}" \ -- To view, visit https://gerrit.ovirt.org/39972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I983b60824e31f279df0fe11c8adeb34b16f56e6a Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ondra Machacek <omach...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Ondřej Macháček <machacek.on...@gmail.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
