Created a review request https://reviews.apache.org/r/19022/. Since this is a small change not creating a separate merge request mail.
-Koushik On 13-Feb-2014, at 2:23 PM, Daan Hoogland <daan.hoogl...@gmail.com> wrote: > Koushik, > > I am afraid I am not. Using English for identifiers there is a semantic > differnce of using <word>+'s' or just <word>. it is the same difference as > using String or Collection<String>. For the sake of quality assurance and > maintainability I call upon you to add a parameter for the use with > multiple instances. Nomenclature matters a lot. There are more of these > little 'English' rules implied in using a programming language, like the > use of adjectives. They matter in Westerns languages and unless we define a > system wide set of different rules let's stick with English. > > kind regards, > > Daan Hoogland > > > Op 13 feb. 2014 08:32 schreef "Koushik Das" <koushik....@citrix.com>: > >> I see that there are 2 different opinions >> >> 1. Use 'id' for multiple ids as well. This is simple and compact and >> nothing breaks. >> 2. Have both 'id' and 'ids'. This can lead to confusion if 'id' is not >> deprecated (breaking change and would require a version change). >> >> Daan, >> Is it ok to move ahead with #1 for now? Later on when there is a version >> change required, this can be revisited. >> >> -----Original Message----- >> From: Daan Hoogland [mailto:daan.hoogl...@gmail.com] >> Sent: Monday, 10 February 2014 5:11 PM >> To: dev >> Subject: Re: [PROPOSAL] List VM API enhancement >> >> I don't like the id that id will be used for a list of ids. I would like >> to see the two both added to the api. They don't even need to bee mutually >> exclusive. the (human) semantics of id and ids is (in >> english) quite different and should be honored. >> >> regards, >> Daan >> >> On Fri, Feb 7, 2014 at 11:24 PM, Min Chen <min.c...@citrix.com> wrote: >>> Yes. >>> >>> -min >>> >>> Sent from my iPhone >>> >>>> On Feb 7, 2014, at 11:10 AM, "Alena Prokharchyk" < >> alena.prokharc...@citrix.com> wrote: >>>> >>>> We can just agree from now on to use the ³id" for handling multiple ids. >>>> And of course, we can never delete the ³ID² parameter just to satisfy >>>> the old convention, as this is the most used parameter :) >>>> >>>> I can see that several existing commands - archive/deleteAlerts are >>>> using ApiConstants.IDs parameter. We can mark IDs as deprecated, so >>>> its no longer used by new commands. >>>> >>>> -Alena. >>>> >>>>> On 2/7/14, 11:03 AM, "Koushik Das" <koushik....@citrix.com> wrote: >>>>> >>>>> Good point Min. >>>>> I also thought about it but looking at some of the existing APIs >>>>> thought of keeping both. >>>>> >>>>> For e.g. in deploy VM api there is a parameter called 'networkids' >>>>> which can take an array of network IDs. Note that the naming >>>>> convention of ending in 's'. Now by this logic we should name the >>>>> parameter 'ids' and remove the existing parameter 'id' which will be >>>>> a breaking change. In case the existing 'id' parameter is used for >>>>> multiple IDs that breaks the parameter naming convention. >>>>> >>>>> I am all in favour of using the existing 'id' parameter if there is >>>>> no issues with breaking the naming convention. >>>>> >>>>> >>>>>> On 07-Feb-2014, at 11:25 PM, Min Chen <min.c...@citrix.com> wrote: >>>>>> >>>>>> Hi Koushik, >>>>>> >>>>>> I agree with the idea of supporting multiple IDs. But I may not >>>>>> like the idea of introducing another different query parameter >>>>>> "ids" for this purpose. Why cannot we just change current "id" >>>>>> parameter to take a list of values? This way, user will not need to >>>>>> use two different parameters for single or multiple cases. >>>>>> Maintaining two different parameters for similar purpose is >>>>>> error-prone. If you look at Amazon EC2 api, you will notice that >>>>>> they are also using the similar convention, id parameter can be one >>>>>> or more. >>>>>> >>>>>> Thanks >>>>>> -min >>>>>> >>>>>>> On 2/6/14 3:24 AM, "Koushik Das" <koushik....@citrix.com> wrote: >>>>>>> >>>>>>> Yes it will be like a findByIds() and the one id case is just a >>>>>>> special case for this. >>>>>>> >>>>>>> On 06-Feb-2014, at 4:24 PM, Daan Hoogland >>>>>>> <daan.hoogl...@gmail.com> >>>>>>> wrote: >>>>>>> >>>>>>>> looks nice, it will be backed by the current query for one id? or >>>>>>>> will you write a findByIds()? >>>>>>>> >>>>>>>> On Thu, Feb 6, 2014 at 9:35 AM, Abhinandan Prateek >>>>>>>> <abhinandan.prat...@citrix.com> wrote: >>>>>>>>> +1, The listVM call is one of the most resource intensive call. >>>>>>>>> +Any >>>>>>>>> step >>>>>>>>> to optimise it are welcome. >>>>>>>>> >>>>>>>>>> On 06/02/14 2:01 pm, "Koushik Das" <koushik....@citrix.com> >> wrote: >>>>>>>>>> >>>>>>>>>> Currently list VM can only be called using a single VM ID. So >>>>>>>>>> if there is a need to query a set of VMs using ID then either >>>>>>>>>> multiple list VM calls need to be made or all VMs needs to be >>>>>>>>>> fetched and then do a client side filtering. Both approaches >>>>>>>>>> are sub-optimal - the former results in multiple queries to >>>>>>>>>> database and the latter will be an overkill if you need a small >>>>>>>>>> subset from a very large number of VMs. >>>>>>>>>> >>>>>>>>>> The proposal is to have an additional parameter to specify a >>>>>>>>>> list of VM IDs for which the data needs to be fetched. Using >>>>>>>>>> this the required VMs can be queried in an efficient manner. >>>>>>>>>> With the new parameter the syntax would look like >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> http://localhost:8096/api?command=listVirtualMachines&listAll=t >>>>>>>>>> rue&id >>>>>>>>>> s= >>>>>>>>>> edd >>>>>>>>>> >>>>>>>>>> ac053-9b12-4d2e-acb7-233de2e98112,009966fc-4d7b-4f84-8609-25497 >>>>>>>>>> 9ba013 >>>>>>>>>> 4 >>>>>>>>>> >>>>>>>>>> The new 'ids' parameter will be mutually exclusive with the >>>>>>>>>> existing 'id' >>>>>>>>>> parameter. >>>>>>>>>> >>>>>>>>>> Let me know if there are any concerns/comments. >>>>>>>>>> >>>>>>>>>> Thanks, >>>>>>>>>> Koushik >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> -- >>>>>>>> Daan >>>> >> >> >> >> -- >> Daan >>
