Hi All, the roodiskresize is no longer valid. as there is no code which is using rootdiskresize currently.
As a part of the custom service offering we had to enable specifying custom values to parameters cpu, memory and cpuCores. instead of adding a parameter for each of these values we changed it use a details map. This will also not require any further changes in the API if we need to add some more custom values in future. On 08-Mar-2014, at 1:42 am, Marcus <shadow...@gmail.com> wrote: > Any suggestion? Do we go forward assuming that the correct parameter > for resize on deploy is: > > deployVirtualMachine&details[0].rootdisksize=3 > > or do we change it to > > deployVirtualMachine&rootdisksize=3 > > On Tue, Mar 4, 2014 at 4:14 PM, Marcus <shadow...@gmail.com> wrote: >> Ok, sounds like there needs to be some work done to make these more >> consistent, perhaps. Can you comment on why rootdisksize was made from >> a parameter into a part of the details map? >> >> On Tue, Mar 4, 2014 at 3:12 AM, Bharat Kumar <bharat.ku...@citrix.com> wrote: >>> Hi ALL, >>> There are many other APIs that use Map like createNetworkOffering, >>> updateZone, createTemplate, in most of the cases we do not >>> say how to use maps, one way would be to write this in the description or to >>> use the same way to access maps of all APIs. >>> >>> BTW the way to use details in deploy vm API is >>> details[0].foo=bar&details[1].baz=12 where foo and baz are keys. >>> >>> Also if we want to use the regix protected static final String >>> MAP_KEY_PATTERN_EXPRESSION = "^([^\\[^\\]]+)\\[(\\d+)\\]\\.key$"; >>> protected static final >>> String MAP_VALUE_PATTERN_EXPRESSION = "^[^\\[^\\]]+\\[\\d+\\]\\.value$"; >>> >>> wil this work in the following case. I believe service is the key here which >>> repeats. >>> http://10.147.59.119:8080/client/api?command=createNetworkOffering&response=json&sessionkey=/kGFJDXFmMQU8JZnnC7QFfj3tV8=&name=bharat&displayText=bharat&guestIpType=Isolated&lbType=publicLb&; >>> servicecapabilitylist[0].service=SourceNat&servicecapabilitylist[0].capabilitytype=SupportedSourceNatTypes& >>> servicecapabilitylist[0].capabilityvalue=peraccount& >>> servicecapabilitylist[1].service=lb&servicecapabilitylist[1].capabilitytype=SupportedLbIsolation& >>> servicecapabilitylist[1].capabilityvalue=dedicated&availability=Optional&egresspolicy=ALLOW&state=Creating&status=Creating&allocationstate=Creating&supportedServices=Vpn,Dhcp,Dns,Firewall,Lb,UserData,SourceNat,StaticNat,PortForwarding&specifyIpRanges=false&specifyVlan=false&isPersistent=false&conservemode=false&serviceProviderList[0].service=Vpn&serviceProviderList[0].provider=VirtualRouter&serviceProviderList[1].service=Dhcp&serviceProviderList[1].provider=VirtualRouter&serviceProviderList[2].service=Dns&serviceProviderList[2].provider=VirtualRouter&serviceProviderList[3].service=Firewall&serviceProviderList[3].provider=VirtualRouter&serviceProviderList[4].service=Lb&serviceProviderList[4].provider=VirtualRouter&serviceProviderList[5].service=UserData&serviceProviderList[5].provider=VirtualRouter&serviceProviderList[6].service=SourceNat&serviceProviderList[6].provider=JuniperSRX&serviceProviderList[7].service=StaticNat&serviceProviderList[7].provider=JuniperSRX&serviceProviderList[8].service=PortForwarding&serviceProviderList[8].provider=JuniperSRX&egressdefaultpolicy=true&traffictype=GUEST&_=1393925230248 >>> >>> >>> >>> On 04-Mar-2014, at 2:30 am, Marcus <shadow...@gmail.com> wrote: >>> >>> Along these lines, the details parameter in deployVirtualMachine seems >>> broken. If I call "details[0].key=foo,details[0].value=bar", it stores >>> entries in the database like this: >>> >>> id | vmid | name | value | display >>> >>> 12 | 25 | value | bar | 1 >>> 13 | 25 | key | foo | 1 >>> >>> It seems as though this might be correct per Alena's email, but I >>> don't understand how this can be reconstructed into foo=bar when >>> there's no binding between the two rows. Perhaps details are supposed >>> to be passed differently than the resource tags, because if I do >>> "details[0].foo=bar&details[1].baz=12", I get: >>> >>> id | vmid | name | value | display >>> >>> 12 | 25 | foo | bar | 1 >>> 13 | 25 | baz | 12 | 1 >>> >>> And indeed there is code utilizing these details already committed >>> that expects this format, as deployVirtualMachines getDetails() only >>> seems to pass a correct Map<String, String> with Key, Value if I use >>> this format. >>> >>> On Mon, Mar 3, 2014 at 11:48 AM, Antonio Fornié Casarrubios >>> <antonio.for...@gmail.com> wrote: >>> >>> Hi Alena, >>> >>> Of course, the API will not have any changes. This is not a functional >>> change, just some refactoring. The problem is there are many things in CS >>> that really need some refactoring otherwise the problem will continue >>> growing more and more, but doing the change and above all making sure it >>> all works afterwards is not simple. >>> >>> I will make sure that everything works exactly the same way and that the >>> data returned is also the same. >>> >>> Thanks. Cheers >>> Antonio >>> >>> >>> 2014-03-03 18:48 GMT+01:00 Alena Prokharchyk <alena.prokharc...@citrix.com>: >>> >>> Antonio, sure I will review the patch. But please make sure that API >>> backwards compatibly is intact, otherwise the fix won¹t be accepted. >>> >>> -Alena. >>> >>> >>> On 3/2/14, 4:31 PM, "Antonio Fornié Casarrubios" >>> <antonio.for...@gmail.com> wrote: >>> >>> Hi Alena, >>> >>> The reasons for this strange format? I don't know. There doesn't seem to >>> be >>> one. After asking on my team and in the dev list I thought perhaps you >>> could know. It seems we all see it strange and nobody knows why. But of >>> course, if it is for reasons I will stop the change. >>> >>> >>> >>> And about the DB, you are right, in the DB is not like I said. But you can >>> have this in a table row field: >>> {0={value=Toronto,key=City}} >>> for some tables. I think there are two cases: >>> >>> 1- params in wich the get method fixes the params on the fly. In these of >>> course the strange format is not propagated anymore. But this is still >>> wrong: the format itself before the get is invoked, the time spent on >>> fixing something that should be a normal Map from the begining (each time >>> the get method is invoked) and mainly the fact that these get methods that >>> fix the map on the fly are copies of each other: instead of fixing the >>> structure in one method, the are plenty of methods almost identical >>> copying >>> and pasting the same lines. Some times the same method twice in the same >>> cmd class for two Map params (look CreateNetworkOfferingCmd >>> #getServiceCapabilities and #getServiceProviders). >>> >>> 2- params in which the get method returns the map as it is. With the >>> strange format. For example, >>> Cloudmonkey command >>> create networkoffering ... tags[0].key="City" tags[0].value="Toronto" >>> >>> You store in the table network_offeringstags, field tags, the String: >>> {0={value=Toronto,key=City}} >>> (including brackets and all) >>> >>> So knowing all this I guess you agree this should be refactored... unless >>> at some point the strange format is needed. But after looking for it >>> everywhere I didn't find any place where it was. I already did the change >>> and tested most of the cases and it all seems to work. >>> >>> >>> It would be great if once I upload the patch somebody could help me double >>> check that it doesn't brake anything, not only reviewing to code. I did >>> plenty of tests of many kinds, but I cannot be sure that I am covering >>> enough. Further, there seem to be several places where the code expects >>> the >>> strange format. >>> ->ConfigurationManagerImpl line 1545 >>> >>> >>> Thanks. Cheers >>> Antonio >>> >>> >>> 2014-02-28 18:44 GMT+01:00 Alena Prokharchyk >>> <alena.prokharc...@citrix.com>: >>> >>> >>> >>> From: Antonio Fornié Casarrubios <antonio.for...@gmail.com> >>> Date: Friday, February 28, 2014 at 2:09 AM >>> To: Rohit Yadav <rohityada...@gmail.com>, cloudstack < >>> dev@cloudstack.apache.org>, Alena Prokharchyk < >>> alena.prokharc...@citrix.com> >>> Subject: Re: [PROPOSAL][QUESTION] Map parameters in API Commands >>> >>> Hi Alena, >>> >>> I would like to know your opinion on this change. Mainly consists on: >>> 1- Change the way we store the Map params after unpackParams in order to >>> have, for each Map param, a Map<String, String> instead of Map<String, >>> Map<String, Object>>. >>> >>> >>> -Antonio, what was the reason for storing the parameter in the old >>> format to begin with? Where there any case where we actually needed a >>> map >>> of map parameters? >>> >>> >>> >>> >>> 2- There are many commands that fix this strange format on demand on >>> their getters, so they do the conversion there. Since I already have the >>> final format I replace these getters with just >>> getTags(){ return this.tags;} >>> >>> 3- Persistence of these Map params. This last change is more tricky and >>> error-prone but the previous two would brake the functionality without >>> it. >>> Actually it doesn't seem that I should change this for all the cases, >>> given >>> that for some commands the current behavior is storing in the DB the >>> Map as >>> it comes, so after the change it will just do the same and thus >>> retrieve it >>> with the right format. So, although in the tables we move from >>> ------ >>> key | City >>> ------ >>> value | The Hague >>> ------ >>> >>> to >>> ------ >>> City | The Hague >>> ------ >>> >>> then in memory, after DB read, we will just have the proper format >>> again >>> (Map<String, String>). Is that right? >>> >>> >>> >>> - in what table do you see key name being a field name? I've looked >>> at >>> various *_details tables, as well as resource_tag table, everywhere >>> we have >>> key/value fields where we store key and the value respectfully: >>> >>> mysql> desc user_Vm_details; >>> >>> +---------+---------------------+------+-----+---------+----------------+ >>> | Field | Type | Null | Key | Default | Extra >>> | >>> >>> +---------+---------------------+------+-----+---------+----------------+ >>> | id | bigint(20) unsigned | NO | PRI | NULL | auto_increment >>> | >>> | vm_id | bigint(20) unsigned | NO | MUL | NULL | >>> | >>> | name | varchar(255) | NO | | NULL | >>> | >>> | value | varchar(1024) | NO | | NULL | >>> | >>> | display | tinyint(1) | NO | | 1 | >>> | >>> >>> +---------+---------------------+------+-----+---------+----------------+ >>> 5 rows in set (0.01 sec) >>> >>> mysql> desc resource_tags; >>> >>> >>> +---------------+---------------------+------+-----+---------+----------- >>> -----+ >>> | Field | Type | Null | Key | Default | Extra >>> | >>> >>> >>> +---------------+---------------------+------+-----+---------+----------- >>> -----+ >>> | id | bigint(20) unsigned | NO | PRI | NULL | >>> auto_increment | >>> | uuid | varchar(40) | YES | UNI | NULL | >>> | >>> | key | varchar(255) | YES | | NULL | >>> | >>> | value | varchar(255) | YES | | NULL | >>> | >>> | resource_id | bigint(20) unsigned | NO | MUL | NULL | >>> | >>> | resource_uuid | varchar(40) | YES | | NULL | >>> | >>> | resource_type | varchar(255) | YES | | NULL | >>> | >>> | customer | varchar(255) | YES | | NULL | >>> | >>> | domain_id | bigint(20) unsigned | NO | MUL | NULL | >>> | >>> | account_id | bigint(20) unsigned | NO | MUL | NULL | >>> | >>> >>> >>> +---------------+---------------------+------+-----+---------+----------- >>> -----+ >>> >>> >>> 4- The last change should be related to any code expecting the old >>> format, that will fail with the new one. I guess UI will be an example >>> of >>> that, but I didn't check yet. If the JS code receives the new Map >>> serialized, then chances are this will break it, right? Can you tell >>> your >>> thoughts on this? Can you tell me places I should check first to confirm >>> this guess? >>> >>> >>> - Its not just about the uI> You should never break the API backwards >>> compatibility. Remember that lots of third party vendors use our APIs, >>> not >>> the UI. As long as we support the old format, introducing the new one >>> shouldn't be a problem. >>> >>> >>> >>> Considering it all, do you think this change is worth it? For me this >>> seems to be something that was wrong from the beginning and it should >>> have >>> been changed before the mess got spread. But know, although I want to >>> fix >>> it, I'm afraid this involves touching too many things in order to fix >>> something that looks horrible but seems to be actually working and I >>> don't >>> want to break. >>> >>> Thanks. Cheers >>> Antonio >>> >>> >>> >>> 2014-02-12 23:32 GMT+01:00 Rohit Yadav <rohityada...@gmail.com>: >>> >>> On Wed, Feb 12, 2014 at 9:52 PM, Antonio Fornié Casarrubios >>> <antonio.for...@gmail.com> wrote: >>> >>> Hi Rohit, >>> >>> I didn't mean changing the format of the HTTP request, but only >>> >>> changing the >>> >>> intermediate format in which we keep it in the property of the >>> >>> Command >>> >>> class. I mentioned the format in the request just to explain what I >>> >>> meant. >>> >>> >>> My proposal is to leave the request format as it is, but then when >>> >>> the >>> >>> method "apiDispatcher#setFieldValue" parses the map and assign it to >>> >>> the >>> >>> property, do it in a normal way: which is a Map<String, String> >>> >>> instead >>> of a >>> >>> Map<String, Map<String, Object>> as it is now. And then, our getter >>> >>> methods >>> >>> (like CreateTagsCommand#GetTag) will be just a normal getter that >>> >>> doesn't >>> >>> need to transform the structure on the fly. >>> >>> >>> Cool, let's request the present API layer maintainer(s) and other >>> folks in the community to comment. >>> >>> Regards. >>> >>> >>> Thanks, cheers >>> antonio >>> >>> >>> 2014-02-11 17:38 GMT+01:00 Rohit Yadav <rohityada...@gmail.com>: >>> >>> Hi Antonio, >>> >>> On Tue, Feb 11, 2014 at 9:57 PM, Antonio Fornié Casarrubios >>> <antonio.for...@gmail.com> wrote: >>> >>> Hi all, >>> >>> When invoking a CS API command that has parameters of type Map, >>> >>> the >>> >>> request >>> will be something like this: >>> >>> >>> >>> URL/api?command=createTags&tags[0].key=region&tags[0].value=canada&tags[ >>> 1].key=name&tags[1].value=bob >>> >>> >>> in order to send a Map with the pairs: >>> >>> tags{ >>> region : "canada", >>> name : "bob" >>> } >>> >>> Then in the server side the parameters go through several stages >>> >>> (IMHO >>> >>> too >>> many), and have different formats. At some point >>> >>> apiDispatcher#setFieldValue >>> >>> will assign the value to the command property (CreateTagsCmd#tag >>> >>> in >>> the >>> >>> example) in a VERY strange way: >>> >>> CreateTagsCmd#tag = { >>> 0 : { >>> "key" : "region", >>> "value" : "canada" >>> }, >>> 1 : { >>> "key" : "name", >>> "value" : "bob" >>> } >>> } >>> >>> This is true for several Cmd classes. And the funny thing is they >>> usually >>> provide a public getter method to get the Map in an already >>> >>> "normalized" >>> >>> structure. The problem is we have this method again a again in >>> >>> each >>> of >>> >>> these commands, only with different name depending on what >>> >>> property >>> the >>> >>> get, and the body is almost copy and pasted. so my next >>> >>> refactoring >>> >>> would >>> be to have a generic method only once in BaseCmd so that all >>> >>> subclasses >>> >>> can >>> reuse it for their Map getters. Pretty obvious, but... >>> >>> >>> This is a well know issue and is such a pain, both for the users of >>> the API to create this API and the programmer who have to put hack >>> >>> at >>> >>> the backend to extract the map. >>> >>> Is it really necessary to have this strange format? Wouldn't it be >>> >>> much >>> >>> better to just store it in a more normal way from the beginning, >>> >>> and >>> >>> have >>> the getters just standard getters? Does it have any use to have >>> >>> those >>> >>> Maps >>> of Maps? >>> >>> >>> Changing the API will break many client so no one attempted it for >>> keeping backward-compatibility I think. >>> >>> The HTTP RFC states that if same keys are sent in param they must be >>> received as an array. For example, /api?q=1&q=2&q=3 should received >>> >>> q >>> >>> = [1,2,3] which is what we're not doing. >>> >>> We should do that and this way we can capture maps using keys and >>> values in order, so for example, >>> /api?q.key1=value1&q.key2=value2&q.key1=value3&q.key2=value4 should >>> >>> be >>> >>> received as as array of maps: [{key1: value1, key2: value2}, >>> {key3:value3, key4: value4}] etc. >>> >>> I think it does not have to be maps of maps, but since our API is >>> query based these ugly hacks were invented. We should definitely get >>> rid of them, and perhaps work on the RESTful API layer, cloud-engine >>> and other good stuff we were talking about more than a year ago and >>> deprecate the present query API over next few years. Thoughts, >>> >>> flames? >>> >>> >>> Regards. >>> >>> >>> Thanks. Cheers >>> Antonio Fornie >>> Schuberg Philis - MCE >>> >>> >>> >>> >>> >>> >>> >>> >>>
