Michael Pasternak has posted comments on this change.

Change subject: engine: watchdog - restapi support
......................................................................


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

(4 inline comments)

couple of tiny comments, al in all - well done.

....................................................
File 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendTemplateResource.java
Line 127:     protected Template doPopulate(Template model, VmTemplate entity) {
Line 128:         return model;
Line 129:     }
Line 130: 
Line 131:     @Override
please add @SingleEntityResource annotation here
Line 132:     public WatchdogsResource getWatchdogsResource() {
Line 133:         return inject(new BackendWatchdogsResource(guid,
Line 134:                 VdcQueryType.GetWatchdog,
Line 135:                 new IdQueryParameters(guid)));


....................................................
File 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmResource.java
Line 137:                                                 
VdcQueryType.GetVmByVmId,
Line 138:                                                 new 
IdQueryParameters(guid)));
Line 139:     }
Line 140: 
Line 141:     @Override
please add @SingleEntityResource annotation here
Line 142:     public WatchdogsResource getWatchdogsResource() {
Line 143:         return inject(new BackendWatchdogsResource(guid,
Line 144:                                                 
VdcQueryType.GetWatchdog,
Line 145:                                                 new 
IdQueryParameters(guid)));


....................................................
File 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendWatchdogsResource.java
Line 102: 
Line 103:     @Override
Line 104:     protected <T> boolean matchEntity(VmWatchdog entity, T id) {
Line 105:         //since there can be only one
Line 106:         return true;
even if it's true, why not just ---> return entity.getId().equals(id); why to 
cut corners?
Line 107:     }
Line 108: 


....................................................
File 
backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/WatchdogMapper.java
Line 22:                 entity.setModel(map(wdModel, null));
Line 23:             }
Line 24:         }
Line 25:         entity.setModel(VmWatchdogType.getByName(model.getModel()));
Line 26:         entity.setId(new Guid(model.getId()));
please use here GuidUtils.asGuid() it returns BAD_REQUEST when new Guid() 
throws illegalargument exception when string ain't convertible to UUID
Line 27:         return entity;
Line 28:     }
Line 29: 
Line 30:     @Mapping(from = VmWatchdog.class, to = WatchDog.class)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ief5b20ecf2221faf900cadfeafe4c71607a9eca3
Gerrit-PatchSet: 41
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Laszlo Hornyak <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: Ori Liel <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[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