Arik Hadas has posted comments on this change.
Change subject: core: patterned pool names support
......................................................................
Patch Set 4: (2 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommonVmPoolWithVmsCommand.java
Line 61: protected Map<Guid, List<DiskImage>> storageToDisksMap;
Line 62: protected Map<Guid, StorageDomain> destStorages = new
HashMap<Guid, StorageDomain>();
Line 63: private boolean _addVmsSucceded = true;
Line 64:
Line 65: private static final Pattern PATTERNED_POOL_NAME_PATTERN =
Pattern.compile("^.*?([" + VmPool.MASK_CHARACTER + "]+).*?$");
I'm not sure I agree, I think that since the usage of this pattern is specific
to the commands that extend CommonVmPoolWithVmsCommand and it's part of the
responsibility of those commands to generate the names of the created VMs (it's
not that they are making different task besides what they supposed to do, their
task is divided to sub-tasks and generate the names for the pool is one of
them), there's no major advantage of extracting it as a util. but I don't think
that extracting it has bad influence either so - DONE
Line 66:
Line 67: /**
Line 68: * Constructor for command creation when compensation is applied
on startup
Line 69: * @param commandId
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/utils/ValidationUtils.java
Line 35: public static final String NULLABLE_MAC_ADDRESS =
"^((\\d|([a-f]|[A-F])){2}:){5}(\\d|([a-f]|[A-F])){2}$|^$";
Line 36: /** Invalid mac address (for now just checking 00:00:00:00:00:00 */
Line 37: public static final String INVALID_NULLABLE_MAC_ADDRESS =
"^(00:){5}00$";
Line 38: /** the mask will be replaced with zero-padded number in the
generated names of the VMs in the pool,
Line 39: * see AddVmPoolWithVmsCommandTest#validateGenerateVmName and
PoolNameValidationTest for valid and invalid expressions of this pattern */
the tests don't cover it?
in PoolNameValidationTest there're valid/invalid examples of pool names with
pattern, and in NameForVmInPoolGeneratorTest threre're transformations of name
with masks to generated names. I think that tests should also explain how to
use the code, if it's not the case then there's a problem - and the tests
should be improved instead of writing what the code is supposed to do in
comments
Line 40: public static final String POOL_NAME_PATTERN = "^[\\p{L}0-9._-]+["
+ VmPool.MASK_CHARACTER + "]*[\\p{L}0-9._-]*$|^[\\p{L}0-9._-]*[" +
VmPool.MASK_CHARACTER + "]*[\\p{L}0-9._-]+$";
Line 41:
Line 42: private static final Validator validator =
Validation.buildDefaultValidatorFactory().getValidator();
Line 43:
--
To view, visit http://gerrit.ovirt.org/11986
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I81a1ffb6324fb728b2584677dd654ac79b679cd8
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas <[email protected]>
Gerrit-Reviewer: Arik Hadas <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
