<div dir="ltr"><div>thanks for the quick reply :)<br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Apr 14, 2014 at 9:23 PM, Julien Danjou <span dir="ltr"><<a href="mailto:julien@danjou.info" target="_blank">julien@danjou.info</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Mon, Apr 14 2014, ZhiQiang Fan wrote:<br>
<br>
> Hi, developers,<br>
><br>
> For fixing bug <a href="https://launchpad.net/bugs/1304886" target="_blank">https://launchpad.net/bugs/1304886</a>, I uploaded a patch<br>
> <a href="https://review.openstack.org/#/c/86501/" target="_blank">https://review.openstack.org/#/c/86501/</a>, when review this patch, there are<br>
> two different kinds of opinions, I don't know which is the better choice,<br>
> so I ask for help here.<br>
><br>
> The patch aims at disallow user specify duplicate alarm ids in combination<br>
> rule to reduce unnecessary alarm evaluate. We can avoid such input in two<br>
> ways:<br>
><br>
> * reject user's request with 400, so force user to provide unique list<br>
> * accept such request but remove duplicate ids for the user<br>
><br>
> the problem is that<br>
> * the server side actually has no error, it just considers the efficiency<br>
> problem (and the efficiency improvement is tiny), so reject the request<br>
> seems a bit rude<br>
> * the user provide a bad request but we accept, which will cause the user<br>
> think he is doing a right thing, although he can check the return result<br>
> and find something different, but from my experience, I usually only check<br>
> response status code, and will not check each attribute carefully.<br>
><br>
> here is an existent example of accept such problematical request which<br>
> provided by Ildiko Vancsa:<br>
> <a href="https://github.com/openstack/ceilometer/blob/master/ceilometer/api/controllers/v2.py#L904" target="_blank">https://github.com/openstack/ceilometer/blob/master/ceilometer/api/controllers/v2.py#L904</a><br>
> or search 'def statistic' in that file if LOC is changed<br>
><br>
> can you provide your opinion, or directly review that patch please?<br>
<br>
</div></div>"Be conservative in what you do, be liberal in what you accept from<br>
others"<br>
<br>
So I would tend to convert the list to a set and not raise any error. If<br>
the user wants to have extra verification, it can compare what it sent<br>
to what it got in response.<br>
<span class="HOEnZb"><font color="#888888"><br>
--<br>
Julien Danjou<br>
;; Free Software hacker<br>
;; <a href="http://julien.danjou.info" target="_blank">http://julien.danjou.info</a><br>
</font></span></blockquote></div><br><br clear="all"><br>-- <br><div dir="ltr"><div>blog: <a href="http://zqfan.github.com" target="_blank">zqfan.github.com</a><br></div>git: <a href="http://github.com/zqfan" target="_blank">github.com/zqfan</a><br>
</div>
</div>