Moti Asayag has posted comments on this change.

Change subject: core: dont fail on empty ranges
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.ovirt.org/#/c/35391/1/backend/manager/tools/src/main/java/org/ovirt/engine/core/config/entity/helper/MacAddressPoolRangesValueHelper.java
File 
backend/manager/tools/src/main/java/org/ovirt/engine/core/config/entity/helper/MacAddressPoolRangesValueHelper.java:

Line 25:     @Override
Line 26:     public ValidationResult validate(ConfigKey key, String value) {
Line 27: 
Line 28:         if (StringUtils.isBlank(value)) {
Line 29:             return new ValidationResult(true);
> So you're saying instead of letting users enforce their own MAC address man
no...you got it wrong.

if users wish to use their own implementation - they can do it even now - as 
the specific user you've mentioned is doing.
this is not a good way to support custom user behavior. Instead, we should have 
a proper way of doing it, hence RFE.

Supporting a blank range is a magic value which used to be invalid and now you 
suggest to assign a new meaning to it. Meaning changing the semantic of mac 
pool ranges post ga. I haven't seen a requirement in the attached bug for that 
and mostly such thing isn't not a blocker.

I'm not sure what are the full implications of adding a value that used not be 
supported in the system at this stage.
Line 30:         }
Line 31: 
Line 32:         try {
Line 33:             List<String[]> rangesBoundaries = 
MacAddressRangeUtils.rangeStringToStringBoundaries(value);


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I314ff9e62d7515eae334c461a67ce609ca188359
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-3.5
Gerrit-Owner: Martin Mucha <[email protected]>
Gerrit-Reviewer: Lior Vernia <[email protected]>
Gerrit-Reviewer: Martin Mucha <[email protected]>
Gerrit-Reviewer: Moti Asayag <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to