Moti Asayag has posted comments on this change.

Change subject: restapi: Add support for vnic profiles
......................................................................


Patch Set 18:

(6 comments)

....................................................
File 
backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml
Line 2356:   request:
Line 2357:     body:
Line 2358:       parameterType: null
Line 2359:       signatures: []
Line 2360:     urlparams: {}
sorry, accidently added it to /api/vnicprofiles/{vnicprofile:id}|rel=get
Line 2361:     headers: {}
Line 2362: - name: /api/vnicprofiles/{vnicprofile:id}|rel=get
Line 2363:   request:
Line 2364:     body:


Line 2392:   request:
Line 2393:     body:
Line 2394:       parameterType: VnicProfile
Line 2395:       signatures:
Line 2396:       - mandatoryArguments: {vnicprofile.network.id: 'xs:string', 
vnicprofile.name: 'xs:string'}
why should i be supporting network by name ? It is not a unique identifier of a 
network. By supporting network name i won't be able to determine which data 
center the network exists in.
Line 2397:         optionalArguments: {vnicprofile.description: 'xs:string', 
vnicprofile.port_mirroring: 'xs:boolean',
Line 2398:         vnicprofile.custom_properties.custom_property--COLLECTION: 
{custom_property.name: 'xs:string', custom_property.value: 'xs:string'}}
Line 2399:     urlparams: {}
Line 2400:     headers:


....................................................
File 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendAssignedVnicProfilesResource.java
Line 33: 
Line 34:     @Override
Line 35:     protected void validateParameters(VnicProfile vnicProfile) {
Line 36:         validateParameters(vnicProfile, "name");
Line 37:         if (!vnicProfile.isSetNetwork() || 
!vnicProfile.getNetwork().isSetId()) {
moved to add()
Line 38:             vnicProfile.setNetwork(new Network());
Line 39:             vnicProfile.getNetwork().setId(networkId);
Line 40:         }
Line 41:     }


....................................................
File 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVnicProfilesResource.java
Line 35:     }
Line 36: 
Line 37:     @Override
Line 38:     protected void validateParameters(VnicProfile vnicProfile) {
Line 39:         validateParameters(vnicProfile, "name", "network.id");
I wouldn't like to be supporting network name. it is not a unique identifier of 
a network in the system. the result of such action is unexpected: you can 
guarantee which network you add to vnic profile to.
Line 40:     }
Line 41: 
Line 42:     @SingleEntityResource
Line 43:     @Override


....................................................
File 
backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/AbstractBackendVnicProfilesResourceTest.java
Line 203:     @Ignore
Line 204:     @Override
Line 205:     public void testListCrashClientLocale() throws Exception {
Line 206:     }
Line 207: 
I wrote an explanation on p.s. 8:

To my understanding the implementation of those tests relies on the search 
engine. The vnic profiles has no search support, therefore they doesn't seem 
relevant for it.

In any case, added tests for it.
Line 208:     protected abstract void setGetAllVnicProfilesExpectations();
Line 209: 
Line 210:     protected void setUpEntityQueryExpectations(int times, int index, 
boolean notFound) throws Exception {
Line 211:         while (times-- > 0) {


....................................................
File 
backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVnicProfilesResourceTest.java
Line 35: 
Line 36:     @Override
Line 37:     protected List<VnicProfile> getCollection() {
Line 38:         return collection.list().getVnicProfiles();
Line 39:     }
the tests (testAdd* and testRemove*) are inherited from the base class 
(AbstractBackendVnicProfilesResourceTest)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7a0d472215ec1b0c359c0e2012e142ae8a627ce9
Gerrit-PatchSet: 18
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Moti Asayag <masa...@redhat.com>
Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com>
Gerrit-Reviewer: Michael Pasternak <mpast...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to