Laszlo Hornyak has posted comments on this change.
Change subject: engine: fix add watchdog
......................................................................
Patch Set 2:
(2 comments)
indeed it messed up the rsdl
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendWatchdogsResource.java
Line 63: @Override
Line 64: protected VdcActionParametersBase getAddParameters(VmWatchdog
entity, WatchDog device) {
Line 65: WatchdogParameters watchdogParameters = new
WatchdogParameters();
Line 66: validateEnums(WatchDog.class, device);
Line 67: watchdogParameters.setAction(getMapper(WatchdogAction.class,
VmWatchdogAction.class).map(WatchdogAction.valueOf(StringUtils.upperCase(device.getAction())),
Done
Line 68: null));
Line 69: watchdogParameters.setModel(getMapper(WatchdogModel.class,
VmWatchdogType.class).map(WatchdogModel.valueOf(StringUtils.upperCase(device.getModel())),
Line 70: null));
Line 71: watchdogParameters.setId(parentId);
Line 65: WatchdogParameters watchdogParameters = new
WatchdogParameters();
Line 66: validateEnums(WatchDog.class, device);
Line 67: watchdogParameters.setAction(getMapper(WatchdogAction.class,
VmWatchdogAction.class).map(WatchdogAction.valueOf(StringUtils.upperCase(device.getAction())),
Line 68: null));
Line 69: watchdogParameters.setModel(getMapper(WatchdogModel.class,
VmWatchdogType.class).map(WatchdogModel.valueOf(StringUtils.upperCase(device.getModel())),
Done
one thing that I do not quite understand here is that (line 66) we validated
the enums, now the difference between valueOf and fromValue is that fromValue
catches the IllegalArgumentException (which may only come from error in
validation) and rather than throwing an exception to prevent further issues, it
returns null, which we send to mapper, that returns null, and then we end up
sending a null in this field to the backend, which will validate and in this
case return an error since it is mandatory field.
Therefore, I think it could have been better with a shorter error flow and in
other cases, when the backend accepts null values (not here) it may be a
serious problem.
So we agreed in fromValue, next patch will be like that.
Line 70: null));
Line 71: watchdogParameters.setId(parentId);
Line 72: watchdogParameters.setVm(isVm(parentId));
Line 73: return watchdogParameters;
--
To view, visit http://gerrit.ovirt.org/17434
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I95362fda69b8ec7c02893ba30cdf19be7eee5eac
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Laszlo Hornyak <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: Ori Liel <[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
