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