Allon Mureinik has posted comments on this change.

Change subject: restapi: add storage server connections resource
......................................................................


Patch Set 23: (8 inline comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/GetStorageServerConnectionByIdQuery.java
Line 13:     @Override
Line 14:     protected void executeQueryCommand() {
Line 15:         getQueryReturnValue().setReturnValue(DbFacade.getInstance().
Line 16:                 getStorageServerConnectionDao().get(getParameters().
Line 17:                           getServerConnectionId()));
This is just a formatting exception - can you remove it from the patch?
Line 18:     }


....................................................
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/StorageServerConnectionQueryParametersBase.java
Line 18:     public StorageServerConnectionQueryParametersBase(String 
serverConnectionId) {
Line 19:         setServerConnectionId(serverConnectionId);
Line 20:     }
Line 21: 
Line 22:     /**used by REST because AbstractBackendResource has id member that 
is always assumed to be guid
Add \n before "used"
s/guid/Guid/, or better yet {@link Guid}
Line 23:      *
Line 24:      * @param serverConnectionId
Line 25:      */
Line 26:     public StorageServerConnectionQueryParametersBase(Guid 
serverConnectionId) {


....................................................
File 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StorageServerConnectionDAODbFacadeImpl.java
Line 23: 
Line 24:     @Override
Line 25:     public List<StorageServerConnections> getAll() {
Line 26:         MapSqlParameterSource parameterSource = 
getCustomMapSqlParameterSource();
Line 27:         return 
getCallsHandler().executeReadList("GetAllstorage_server_connections", 
mapper,parameterSource) ;
please remove the whitespace before ";"
Line 28:     }
Line 29: 
Line 30:     @Override
Line 31:     public StorageServerConnections get(String id) {


....................................................
File 
backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/StorageServerConnectionsResource.java
Line 24:     /**
Line 25:      * Adds a storage connection entity
Line 26:      *
Line 27:      * @param storageConnection  the storage connection to add
Line 28:      * @return        the new newly added storage connection
why all these spaces?
Line 29:      */
Line 30:     @POST
Line 31:     @Formatted
Line 32:     @Consumes({ApiMediaType.APPLICATION_XML, 
ApiMediaType.APPLICATION_JSON, ApiMediaType.APPLICATION_X_YAML})


....................................................
File 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendStorageServerConnectionResource.java
Line 64:                             ? getEntity(VDS.class,
Line 65:                                     SearchType.VDS,
Line 66:                                     "Hosts: name=" + 
host.getName()).getId()
Line 67:                             : null;
Line 68:     }
This isn't indented correctly, please use the formatter.
Line 69:     }


....................................................
File 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendStorageServerConnectionsResource.java
Line 68:             validateParameters(storage, "path");
Line 69:             break;
Line 70:         case POSIXFS:
Line 71:         case GLUSTERFS:
Line 72:             validateParameters(storage, "path", "vfsType");
Are you sure GLUSTERFS hsa vdsType?
Line 73:             break;
Line 74:         default:
Line 75:             break;
Line 76:         }


....................................................
File 
backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/StorageDomainMapper.java
Line 12: import org.ovirt.engine.api.restapi.model.StorageFormat;
Line 13: import org.ovirt.engine.api.restapi.utils.GuidUtils;
Line 14: import 
org.ovirt.engine.core.common.businessentities.StorageDomainStatic;
Line 15: import 
org.ovirt.engine.core.common.businessentities.StorageServerConnections;
Line 16: 
There's a lot of noise in this file.
Can you apply the formatter in a separate patch?
Line 17: public class StorageDomainMapper {
Line 18: 
Line 19:     @Mapping(from = StorageDomain.class, to = 
StorageDomainStatic.class)
Line 20:     public static StorageDomainStatic map(StorageDomain model, 
StorageDomainStatic template) {


....................................................
Commit Message
Line 19: Url is the same as item 4, different request type.
Line 20: 6. Add ability to get a specific connection by its id.
Line 21: Url is the same as item 4.
Line 22: 7. REST unitests
Line 23: 
Please add a link to the feature page
Line 24: Change-Id: If6bc32ead098390723825872f6fb292097d52835


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If6bc32ead098390723825872f6fb292097d52835
Gerrit-PatchSet: 23
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: Daniel Erez <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: Ori Liel <[email protected]>
Gerrit-Reviewer: Tal Nisan <[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