Mike Kolesnik has posted comments on this change. Change subject: engine: Validate network provider properties ......................................................................
Patch Set 4: Code-Review-1 (4 comments) http://gerrit.ovirt.org/#/c/29520/4/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/NetworkProviderValidator.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/NetworkProviderValidator.java: Line 9: import org.ovirt.engine.core.compat.Guid; Line 10: import org.ovirt.engine.core.dal.dbbroker.DbFacade; Line 11: import org.ovirt.engine.core.dao.provider.ProviderDao; Line 12: Line 13: public class NetworkProviderValidator { This could extend ProviderValidator, giving you already validation that provider exists.. Also please write the validations using the new fluent API. Line 14: private Guid providerId; Line 15: private String mappings; Line 16: private Provider<?> provider; Line 17: Line 32: ? ValidationResult.VALID Line 33: : new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_PROVIDER_TYPE_MISMATCH); Line 34: } Line 35: Line 36: public ValidationResult networkMappingsIsSet() { I think mappings should be sent just as part of this command, not in the ctor. Also s/Is/Are/ Line 37: return StringUtils.isBlank(mappings) Line 38: && StringUtils.isBlank(getProviderProperties().getNetworkMappings()) Line 39: ? new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_MISSING_NETWORK_MAPPINGS) Line 40: : ValidationResult.VALID; Line 47: ? new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_MISSING_MESSAGING_BROKER_PROPERTIES) Line 48: : ValidationResult.VALID; Line 49: } Line 50: Line 51: public AgentConfiguration getProviderProperties() { Shouldn't this be called getAgentConfiguration ? Line 52: OpenstackNetworkProviderProperties properties = Line 53: (OpenstackNetworkProviderProperties) provider.getAdditionalProperties(); Line 54: return properties.getAgentConfiguration(); Line 55: } http://gerrit.ovirt.org/#/c/29520/4/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java: Line 548: ACTION_TYPE_FAILED_ISCSI_BOND_NETWORK_CANNOT_BE_REQUIRED(ErrorType.NOT_SUPPORTED), Line 549: ACTION_TYPE_FAILED_PROVIDER_NETWORKS_USED(ErrorType.CONFLICT), Line 550: ACTION_TYPE_FAILED_PROVIDER_TYPE_MISMATCH(ErrorType.BAD_PARAMETERS), Line 551: ACTION_TYPE_FAILED_MISSING_NETWORK_MAPPINGS(ErrorType.BAD_PARAMETERS), Line 552: ACTION_TYPE_FAILED_MISSING_MESSAGING_BROKER_PROPERTIES(ErrorType.BAD_PARAMETERS), I think this should be ErrorType.CONFLICT Line 553: ACTION_TYPE_FAILED_EXTERNAL_NETWORK_CANNOT_BE_PORT_MIRRORED(ErrorType.NOT_SUPPORTED), Line 554: ACTION_TYPE_FAILED_EXTERNAL_NETWORK_CANNOT_BE_REWIRED(ErrorType.NOT_SUPPORTED), Line 555: ACTION_TYPE_FAILED_EXTERNAL_NETWORK_CANNOT_HAVE_MTU(ErrorType.NOT_SUPPORTED), Line 556: ACTION_TYPE_FAILED_EXTERNAL_NETWORKS_CANNOT_BE_PROVISIONED(ErrorType.NOT_SUPPORTED), -- To view, visit http://gerrit.ovirt.org/29520 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idcdcddb9694426c1e656c9fa1e04dc7f37e8115c Gerrit-PatchSet: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Moti Asayag <[email protected]> Gerrit-Reviewer: Mike Kolesnik <[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
