Michael Pasternak has posted comments on this change.

Change subject: restapi: add connections subresource for domain
......................................................................


Patch Set 6: I would prefer that you didn't submit this

(7 inline comments)

....................................................
File 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendStorageDomainServerConnectionResource.java
Line 22:                 new 
StorageServerConnectionQueryParametersBase(guid.toString()));
Line 23:     }
Line 24: 
Line 25:     @Override
Line 26:     public Storage update(Storage connection) {
please remove it from the interface/split it to two if update is not relevant 
in this context,

cause running HEAD on this URI will return PUT and if someone will invoke it, 
he will get UnsupportedOperationException - this doesn't make sense
Line 27:         throw new UnsupportedOperationException();
Line 28:     }
Line 29: 
Line 30:     @Override


....................................................
File 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendStorageDomainServerConnectionsResource.java
Line 24:     }
Line 25: 
Line 26:     @Override
Line 27:     public StorageConnections list() {
Line 28:         List<StorageServerConnections> connections = getConnections();
you can save this local "connections" variable as it not used
Line 29:         return mapCollection(connections);
Line 30:     }
Line 31: 
Line 32:     protected List<StorageServerConnections> getConnections() {


Line 52:         return collection;
Line 53:     }
Line 54: 
Line 55:     @Override
Line 56:     public Response add(Storage storage) {
please remove it from the interface/split it to two if add is not relevant in 
this context,

cause running HEAD on this URI will return POST and if someone will invoke it, 
he will get UnsupportedOperationException - this doesn't make sense
Line 57:         throw new UnsupportedOperationException();
Line 58:     }
Line 59: 
Line 60:     @Override


Line 57:         throw new UnsupportedOperationException();
Line 58:     }
Line 59: 
Line 60:     @Override
Line 61:     public Response remove(String id, Host host) {
please remove it from the interface/split it to two if remove is not relevant 
in this context,

cause running HEAD on this URI will return DELETE and if someone will invoke 
it, he will get UnsupportedOperationException - this doesn't make sense
Line 62:         throw new UnsupportedOperationException();
Line 63:     }
Line 64: 
Line 65:     @Override


....................................................
File 
backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendStorageDomainServerConnectionsResourceTest.java
Line 32:     public void testQuery() throws Exception {
Line 33:     }
Line 34: 
Line 35:     @Test
Line 36:     public void testAddNotSupported() throws Exception {
this test is redundant as UnsupportedOperationException should not be exposed 
at all
Line 37:         setUriInfo(setUpBasicUriExpectations());
Line 38:         control.replay();
Line 39:         Response response = null;
Line 40:         try {


Line 44:         }
Line 45:     }
Line 46: 
Line 47:     @Test
Line 48:     public void testDeleteNotSupported() throws Exception {
same
Line 49:         setUriInfo(setUpBasicUriExpectations());
Line 50:         control.replay();
Line 51:         Response response = null;
Line 52:         try {


....................................................
Commit Message
Line 13: api/storagedomains/<domainId>/storageconnections
Line 14: and:
Line 15: api/storagedomains/<domainId>/storageconnections/<connectionId>
Line 16: Exposing GET only, no add/delete/update.
Line 17: 
1. ^ and BZ if exist

2. please update rsdl_metadata.yaml accordingly

thanks.
Line 18: Change-Id: I0f63f8384c245eb28c18a3fde87ba57a8c1cd678


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0f63f8384c245eb28c18a3fde87ba57a8c1cd678
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to