Yair Zaslavsky has posted comments on this change.

Change subject: core: Adds custom properties to VmDevice
......................................................................


Patch Set 5: (4 inline comments)

....................................................
File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
Line 171: ACTION_TYPE_FAILED_DISK_LUN_HAS_NO_VALID_TYPE=Cannot ${action} 
${type}. The provided lun has no valid lun type.
Line 172: ACTION_TYPE_FAILED_DISK_LUN_ISCSI_MISSING_CONNECTION_PARAMS=Cannot 
${action} ${type}. The provided lun is missing at least one connection 
parameter (address/port/iqn).
Line 173: ACTION_TYPE_FAILED_MIGRATION_IN_PROGRESS=Cannot ${action} ${type}. VM 
migration is in progress
Line 174: ACTION_TYPE_FAILED_MIGRATION_TO_SAME_HOST=Cannot ${action} ${type}. 
source and destination is the same.
Line 175: ACTION_TYPE_FAILED_INVALID_CUSTOM_PROPERTIES_INVALID_SYNTAX=Cannot 
${action} ${type} if custom properties are in invalid format. Please check the 
input.
Martin, Can you also provide a fix for AppErrors.properties at frontend as well?

frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java

And the relevant AppErrors.properties at frontend as well

Thanks!Martin, Can you also provide a fix for AppErrors.properties at frontend 
as well?

frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java

And the relevant AppErrors.properties at frontend as well

Thanks!
Line 176: ACTION_TYPE_FAILED_INVALID_CUSTOM_PROPERTIES_INVALID_KEYS=Cannot 
${action} ${type} if some of the specified custom properties are not configured 
by the system. The keys are: ${MissingKeys}
Line 177: ACTION_TYPE_FAILED_INVALID_CUSTOM_PROPERTIES_INVALID_VALUES=Cannot 
${action} ${type} if some of the specified custom properties have illegal 
values. The keys are: ${WrongValueKeys}
Line 178: ACTION_TYPE_FAILED_CUSTOM_PROPERTIES_NOT_SUPPORTED_IN_VERSION=Cannot 
${action} ${type} custom properties are not supported in version: 
${NotSupportedInVersion}
Line 179: ACTION_TYPE_FAILED_INVALID_DEVICE_TYPE_FOR_CUSTOM_PROPERTIES=Cannot 
${action} ${type} custom properties are not supported for device type: 
${InvalidDeviceType}


....................................................
File 
backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/customprop/DevicePropertiesUtilsTest.java
Line 122:         DevicePropertiesUtils utils = mockDevicePropertiesUtils();
Line 123:         List<ValidationError> errors = 
utils.validateDeviceProperties(Version.v3_3, device);
Line 124: 
Line 125:         assertFalse(errors.isEmpty());
Line 126:         assertTrue(errors.get(0).getReason() == 
ValidationFailureReason.SYNTAX_ERROR);
Don't you prefer to use assertEquals?
Line 127:     }
Line 128: 
Line 129:     /**
Line 130:      * Tries to set invalid property value to a device with supported 
version


Line 415:     /**
Line 416:      * Tests converting device properties to a map
Line 417:      */
Line 418:     @Test
Line 419:     public void convertDevicePropertiesToMap() {
Thanks for the thorough test.
Line 420:         String customDevPropSpec = 
"{type=interface;prop={speed=[0-9]{1,5};duplex=^(full|half)$}}";
Line 421: 
Line 422:         // mock DevicePropertiesUtils
Line 423:         DevicePropertiesUtils utils = 
mockDevicePropertiesUtils(customDevPropSpec);


....................................................
File 
backend/manager/tools/src/main/java/org/ovirt/engine/core/config/entity/helper/DeviceCustomPropertiesValueHelper.java
Line 16:     public ValidationResult validate(ConfigKey key, String value) {
Line 17:         boolean result = true;
Line 18:         String errMsg = null;
Line 19: 
Line 20:         if (isCustomDevicePropertiesSupported(key.getVersion())) {
Good , this makes it more clear
Line 21:             if 
(!DevicePropertiesUtils.getInstance().isDevicePropertiesDefinitionValid(value)) 
{
Line 22:                 result = false;
Line 23:                 errMsg =
Line 24:                         "Invalid syntax, custom device properties 
specification should conform to "


--
To view, visit http://gerrit.ovirt.org/14814
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I07daa5cccbf8e3cbd6b4191bbc8ebf0729d9d9a0
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Peřina <mper...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Martin Peřina <mper...@redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to