Michael Pasternak has posted comments on this change.

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


Patch Set 30: (7 inline comments)

....................................................
File 
backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml
Line 3371:       parameterType: null
Line 3372:       signatures: []
Line 3373:     urlparams: {}
Line 3374:     headers:
Line 3375:       Content-Type: {value: application/xml|json, required: true}
Content-Type is needed when you sending any content to the server, while it's 
not your case
Line 3376: 
Line 3377: - name: /api/storageconnections/{storageconnection:id}|rel=get
Line 3378:   request:
Line 3379:     body:


Line 3380:       parameterType: null
Line 3381:       signatures: []
Line 3382:     urlparams: {}
Line 3383:     headers:
Line 3384:       Content-Type: {value: application/xml|json, required: true}
same
Line 3385: 
Line 3386: - name: /api/storageconnections/{storageconnection:id}|rel=delete
Line 3387:   request:
Line 3388:     body:


Line 3389:       parameterType: null
Line 3390:       signatures: []
Line 3391:     urlparams: {}
Line 3392:     headers:
Line 3393:       Content-Type: {value: application/xml|json, required: true}
1. same

2. please add Correlation-Id
Line 3394: 
Line 3395: - name: /api/storageconnections/{storageconnection:id}|rel=update
Line 3396:   request:
Line 3397:     body:


Line 3409:       - mandatoryArguments: {}
Line 3410:         optionalArguments:  {storageconnection.path: 'xs:string'}
Line 3411:     urlparams: {}
Line 3412:     headers:
Line 3413:       Content-Type: {value: application/xml|json, required: true}
please add Correlation-Id
Line 3414: 
Line 3415: - name: /api/storageconnections|rel=add
Line 3416:   request:
Line 3417:     body:


Line 3420:       - mandatoryArguments: {storageconnection.address: 'xs:string', 
storageconnection.type: 'xs:string', storageconnection.iqn: 'xs:string', 
storageconnection.port: 'xs:int'}
Line 3421:         optionalArguments:  {storageconnection.username: 
'xs:string', storageconnection.password: 'xs:string'}
Line 3422:       - mandatoryArguments: {storageconnection.address: 'xs:string', 
storageconnection.type: 'xs:string', storageconnection.path: 'xs:string'}
Line 3423:         optionalArguments:  {storageconnection.nfs_timeo: 
'xs:string', storageconnection.nfs_version: 'xs:string', 
storageconnection.nfs_retrans: 'xs:string'}
Line 3424:       - mandatoryArguments: {storageconnection.type: 'xs:string', 
storageconnection.path: 'xs:string', storageconnection.vfs_type: 'xs:string'}
shouldn't storageconnection.address appear in mandatory?
Line 3425:         optionalArguments:  {storageconnection.address: 'xs:string', 
storageconnection.mount_options: 'xs:string'}
Line 3426:       - mandatoryArguments: {storageconnection.type: 'xs:string', 
storageconnection.path: 'xs:string'}
Line 3427:         optionalArguments:  {}
Line 3428:     headers:


Line 3426:       - mandatoryArguments: {storageconnection.type: 'xs:string', 
storageconnection.path: 'xs:string'}
Line 3427:         optionalArguments:  {}
Line 3428:     headers:
Line 3429:       Content-Type: {value: application/xml|json, required: true}
Line 3430:       Expect: {value: 201-created, required: false}
please add Correlation-Id


....................................................
File 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendStorageServerConnectionsResource.java
Line 42:         StorageConnections collection = new StorageConnections();
Line 43:         for 
(org.ovirt.engine.core.common.businessentities.StorageServerConnections entity 
: entities) {
Line 44:             Storage connection = map(entity);
Line 45:             if (connection != null) {
Line 46:                 
collection.getStorageConnections().add(addLinks(connection));
please add call to populate: 

addLinks(populate(connection))
Line 47:             }
Line 48:         }
Line 49:         return collection;
Line 50:     }


-- 
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: 30
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