Arik Hadas has posted comments on this change. Change subject: core: change return value of AddDisk ......................................................................
Patch Set 2: (1 comment) http://gerrit.ovirt.org/#/c/36425/2/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java: Line 452: Line 453: if (tmpRetValue.getActionReturnValue() != null) { Line 454: DiskImage diskImage = (DiskImage) tmpRetValue.getActionReturnValue(); Line 455: addDiskPermissions(diskImage); Line 456: getReturnValue().setActionReturnValue(isExecutedAsChildCommand() ? diskImage : diskImage.getId()); > I'm not against changing the return value, but if we do change it i think i if it was easy to change the return value I would do it. it's just that it is also called from rest api where they have an infra that expects to get ID and fetch the entity. I agree that ideally the return value should be the same in all flows, but in this case there are additional constraints.. what is the probability of existing internal call will be changed to external call? it will be a problem but that's not realistic.. Line 457: } Line 458: getReturnValue().setFault(tmpRetValue.getFault()); Line 459: setSucceeded(tmpRetValue.getSucceeded()); Line 460: } -- To view, visit http://gerrit.ovirt.org/36425 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6a41d57bfb1bf48f0a2c6a3703e612b27da50db8 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Arik Hadas <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Arik Hadas <[email protected]> Gerrit-Reviewer: Liron Aravot <[email protected]> Gerrit-Reviewer: Maor Lipchuk <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
