Lior Vernia has posted comments on this change.

Change subject: engine: Implement HostNetworkQosValidator
......................................................................


Patch Set 3:

(3 comments)

http://gerrit.ovirt.org/#/c/34123/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelper.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelper.java:

Line 257:                 }
Line 258: 
Line 259:                 NetworkQosValidator qosValidator = new 
NetworkQosValidator(iface.getQos());
Line 260:                 if (qosValidator.requiredValuesPresent() != 
ValidationResult.VALID) {
Line 261:                     
addViolation(VdcBllMessages.ACTION_TYPE_FAILED_HOST_NETWORK_QOS_SETUP_NETWORKS_MISSING_VALUES,
> Why are you overriding the error message returned from the validator? Is th
This is effectively dead code as the feature has been dormant since 3.4, there 
should be no harm in changing the text (although I agree it's confusing to do 
this here and not in a later patch - just tried to do as much work as possible 
prior to actually changing existing references to NetworkQoS).
Line 262:                             iface.getNetworkName());
Line 263:                 }
Line 264:                 if (qosValidator.peakConsistentWithAverage() != 
ValidationResult.VALID) {
Line 265:                     
addViolation(VdcBllMessages.ACTION_TYPE_FAILED_HOST_NETWORK_QOS_SETUP_NETWORKS_INCONSISTENT_VALUES,


Line 261:                     
addViolation(VdcBllMessages.ACTION_TYPE_FAILED_HOST_NETWORK_QOS_SETUP_NETWORKS_MISSING_VALUES,
Line 262:                             iface.getNetworkName());
Line 263:                 }
Line 264:                 if (qosValidator.peakConsistentWithAverage() != 
ValidationResult.VALID) {
Line 265:                     
addViolation(VdcBllMessages.ACTION_TYPE_FAILED_HOST_NETWORK_QOS_SETUP_NETWORKS_INCONSISTENT_VALUES,
> You're still using the NetworkQosValidator (and not HostNetworkQosValidator
Same.
Line 266:                             iface.getNetworkName());
Line 267:                 }
Line 268:             }
Line 269:         }


http://gerrit.ovirt.org/#/c/34123/3/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/HostNetworkQosValidatorTest.java
File 
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/HostNetworkQosValidatorTest.java:

Line 24:     private HostNetworkQosValidator nullValidator;
Line 25: 
Line 26:     @Before
Line 27:     public void setup() {
Line 28:         qos = mock(HostNetworkQos.class);
> You can add @RunWith(MockitoJUnitRunner.class) to the class, and then you"l
Done
Line 29:         validator = new HostNetworkQosValidator(qos);
Line 30:         nullValidator = new HostNetworkQosValidator(null);
Line 31:     }
Line 32: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibf156e27381111efb3d9abac3e6e37ccdd8f7643
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Lior Vernia <[email protected]>
Gerrit-Reviewer: Alona Kaplan <[email protected]>
Gerrit-Reviewer: Lior Vernia <[email protected]>
Gerrit-Reviewer: Yevgeny Zaspitsky <[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