Einav Cohen has posted comments on this change. Change subject: restapi: default version to 'general' ......................................................................
Patch Set 3: > if you want to force users specifying version, leave only one constructor we cannot do that, as the class is marked as Serializable [since it is being serialized/deserialized between client (javascript) and server], therefore it must contain a default c'tor. > and replace 'String version' with 'Version version', not sure how it will help for our issue: if Version can be set as 'null' - it doesn't help; if Version cannot be set as 'null', then again - we will only cover-up the problem (unless implementing *along with* your c'tor suggestion above, which again - is impossible). > in addition maybe we should add validation to make sure version get specified, if you mean that the GetConfigurationValue query should *fail* in case a 'null' Version value has been provided: That is a great suggested and indeed can help, however can result in many regressions. So before doing that - need to make sure first that we are passing a non-null version in the existing GetConfigurationValue calls in the code (in order to avoid these regressions as much as we can), which is what this patch is all about. > current solution is only temporary resolution of the problem as you have no > tool to prevent someone passing NULL as version tomorrow. Indeed; I am all for the validation solution that you suggested (if I understood you correctly), but it can (and should, IMO) be handled in a separate patch; in any case - no matter how we decide to ultimately solve it - need to make sure first that the GetConfigurationValue calls provide a non-null version value. -- To view, visit http://gerrit.ovirt.org/11173 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I946c9925f3e0b94a9dad86f0958c2f9e1d0ab5c8 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Einav Cohen <eco...@redhat.com> Gerrit-Reviewer: Michael Pasternak <mpast...@redhat.com> Gerrit-Reviewer: Ori Liel <ol...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches