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

Reply via email to