Moti Asayag has posted comments on this change.

Change subject: engine: Validate network provider properties
......................................................................


Patch Set 4:

(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 pro
Done
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 ct
Done
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 ?
Done
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
Done
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

Reply via email to