Alissa Bonas has posted comments on this change. Change subject: core: support adding template version ......................................................................
Patch Set 3: (4 comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java Line 319: if (getParameters().getBaseTemplateId() != null) { Line 320: VmTemplate baseTemplate = getVmTemplateDAO().get(getParameters().getBaseTemplateId()); Line 321: if (baseTemplate == null) { Line 322: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_TEMPLATE_DOES_NOT_EXIST); Line 323: } else if (!baseTemplate.getBaseTemplateId().equals(baseTemplate.getId())) { this is a confusing expression... the same object named baseTemplate is taken here, extract from it both baseTemplateId and id (that sound the same since the object is itself named baseTemplate) so it's not very clear what this code is trying to do. Line 324: // currently template version cannot be base template Line 325: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_TEMPLATE_VERSION_CANNOT_BE_BASE_TEMPLATE); Line 326: Line 327: } Line 321: if (baseTemplate == null) { Line 322: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_TEMPLATE_DOES_NOT_EXIST); Line 323: } else if (!baseTemplate.getBaseTemplateId().equals(baseTemplate.getId())) { Line 324: // currently template version cannot be base template Line 325: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_TEMPLATE_VERSION_CANNOT_BE_BASE_TEMPLATE); why it can't be? it's not clear logic wise. please explain. Line 326: Line 327: } Line 328: } Line 329: .................................................... File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties Line 215: ACTION_TYPE_FAILED_CANNOT_REMOVE_ACTIVE_IMAGE=Cannot ${action} ${type}. Removing the VM active Snapshot is not allowed. Line 216: ACTION_TYPE_FAILED_PROBLEM_WITH_CANDIDATE_INFO=Cannot ${action} ${type}. Failed to get data for Import operation.\n\ Line 217: - Check your Import Domain. Line 218: ACTION_TYPE_FAILED_TEMPLATE_DOES_NOT_EXIST=Cannot ${action} ${type}. The relevant Template doesn't exist. Line 219: ACTION_TYPE_FAILED_TEMPLATE_VERSION_CANNOT_BE_BASE_TEMPLATE=Cannot ${action} ${type}. Template version cannot be used as base for other template version, please use base templates only. I'd change the message as follows: 1. Template version cannot be used ---> This template version cannot be used.... 2. for other template version --> for another template's version. 3. It 's not clear from the message why it cannot be used as base, perhaps an explanation will ease on user not to make this mistake again. Line 220: ACTION_TYPE_FAILED_INSTANCE_TYPE_DOES_NOT_EXIST=Cannot ${action} ${type}. The relevant Instance Type doesn't exist. Line 221: ACTION_TYPE_FAILED_IMAGE_TYPE_DOES_NOT_EXIST=Cannot ${action} ${type}. The relevant Image Type doesn't exist. Line 222: ACTION_TYPE_FAILED_TEMPLATE_IS_DISABLED=Cannot ${action} ${type}. The Template is disabled, please try to enable the template first and try again. Line 223: ACTION_TYPE_FAILED_IMAGE_ALREADY_EXISTS=Cannot ${action} ${type}. One of the Template Images already exists. .................................................... File frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties Line 211: ACTION_TYPE_FAILED_CANNOT_REMOVE_ACTIVE_IMAGE=Cannot ${action} ${type}. Removing the VM active Snapshot is not allowed. Line 212: ACTION_TYPE_FAILED_PROBLEM_WITH_CANDIDATE_INFO=Cannot ${action} ${type}. Failed to get data for Import operation.\n\ Line 213: - Check your Import Domain. Line 214: ACTION_TYPE_FAILED_TEMPLATE_DOES_NOT_EXIST=Cannot ${action} ${type}. The relevant Template doesn't exist. Line 215: ACTION_TYPE_FAILED_TEMPLATE_VERSION_CANNOT_BE_BASE_TEMPLATE=Cannot ${action} ${type}. Template version cannot be used as base for other template version, please use base templates only. Is this relevant for userportal at all? Line 216: ACTION_TYPE_FAILED_INSTANCE_TYPE_DOES_NOT_EXIST=Cannot ${action} ${type}. The relevant Instance Type doesn't exist. Line 217: ACTION_TYPE_FAILED_IMAGE_TYPE_DOES_NOT_EXIST=Cannot ${action} ${type}. The relevant Image Type doesn't exist. Line 218: ACTION_TYPE_FAILED_TEMPLATE_IS_DISABLED=Cannot ${action} ${type}. The Template is disabled, please try to enable the template first and try again. Line 219: ACTION_TYPE_FAILED_IMAGE_ALREADY_EXISTS=Cannot ${action} ${type}. One of the Template Images already exists. -- To view, visit http://gerrit.ovirt.org/22738 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id256ade6e3973d5ad00daa267f80e5ce42b87872 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Alissa Bonas <abo...@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