Shireesh Anjal has posted comments on this change. Change subject: engine: Fix for Exception on adding new host ......................................................................
Patch Set 1: No score This method throws an exception because it is used from other places also (during execution of the command). So instead of checking null conditions from all such places, I thought it is better to throw an exception, expecting it to be automatically handled by the framework. However it seems that such exceptions are not handled by the framework as of now. (InternalCanDoAction does it, but adds a very generic error message CAN_DO_ACTION_GENERAL_FAILURE) I think it would be nice if we catch exceptions in ExecuteAction/InternalCanDoAction and set a VdcFault object on the return value (in case of ExceuteAction), or add a canDoActionMessage with proper code/message from the original exception (in case of InternalCanDoAction). In case we don't want to pass on *ALL* exceptions to UI in such fashion, a new runtime exception class (say ValidationException) can be introduced, and only these exceptions can be handled in this way. It can avoid a lot of boilerplate null checking code. If you like this approach, we will send a patch with such changes. If not, will change the method to return null and null checks in all affected places :) -- To view, visit http://gerrit.ovirt.org/5149 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7828145735aa12cd69fa361b947858b535838024 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Dhandapani Gopal <dgo...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Selvasundaram <sesub...@redhat.com> Gerrit-Reviewer: Shireesh Anjal <san...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
