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

Reply via email to