Kishan, 1. Why Long instead of Integer : You replied that it should be Integer 2. @Encrypt : Does it both encrypt & decrypt? Is there anything necessary to make it work because it doesn't seem to work when I trace the persist.
Thanks Alex Ough On Fri, Jun 27, 2014 at 7:39 AM, Kishan Kavala <kishan.kav...@citrix.com> wrote: > Alex, > > Are the questions on review board? > > > > *From:* Alex Ough [mailto:alex.o...@sungardas.com] > *Sent:* Friday, 27 June 2014 12:03 AM > *To:* Alena Prokharchyk > *Cc:* Kishan Kavala; dev@cloudstack.apache.org; Murali Reddy; Ram Ganesh; > Animesh Chaturvedi > *Subject:* Re: Review Request 20099: Domain-Account-User Sync Up Among > Multiple Regions (Core Changes) > > > > Alena, > > > > It has been reduced almost twice because a lot has been separated from the > CS and moved to the plug-in not because they are 'unnecessary'. Please > remember that my initial implementation was inside the CS not as a plug-in > as I said in the previous email. > > > > Of course, I asked and urged the review repeatedly and you'll see the all > the histories of them if you find emails using this subject, which started > 10/17/13. > > [DISCUSS] Domain-Account-User Sync Up Among Multiple Regions > > Even if I asked so many times, unfortunately, I couldn't get an actual > feedback until Daan finally asked Chiradeep and you to review them, which > is 3/10/14. > > > > Kishan, > > I posted 2 questions, so please guide me for the questions. > > > > Thanks > > Alex Ough > > > > > > On Thu, Jun 26, 2014 at 12:57 PM, Alena Prokharchyk < > alena.prokharc...@citrix.com> wrote: > > Alex, > > > > By “huge” I’ve meant that there was a lot of repetitive hardcoded things, > lot of unnecessary changes to the CS orchestration layer. If you compare a > number of changes now and originally, you can see that it reduced almost > twice. > > > > But lets discuss the complains about lack of initial review as its more > important question. > > > > Review of the design spec should happen before you start designing/coding. > As I jumped on review much later, after you’ve submitted the entire plugin > code, so I I didn’t participate in “Feature Request” discussion review that > might have happened earlier. And I do assume that the reviews/emails > exchanges were done at that initial phase? You should have contacted the > people participating in the initial phase, and ask them for the review as > well. > > > > As a part of my review, I’ve made sure to cover the things I’m certain > should have been changed. I’ve reviewed the feature logic as well, > consulting the FS you’ve written. I’m not saying that there is anything > wrong with your initial design, but asking for a second opinion from the > guys who have more expertise in Regions. > > > > Kishan, please help to do the final review the Alex’s plugin design > https://reviews.apache.org/r/17790 > > > > Thank you, > > Alena. > > *From: *Alex Ough <alex.o...@sungardas.com> > *Date: *Wednesday, June 25, 2014 at 9:03 PM > > > *To: *Alena Prokharchyk <alena.prokharc...@citrix.com> > *Cc: *Kishan Kavala <kishan.kav...@citrix.com>, "dev@cloudstack.apache.org" > <dev@cloudstack.apache.org>, Murali Reddy <murali.re...@citrix.com>, Ram > Ganesh <ram.gan...@citrix.com>, Animesh Chaturvedi < > animesh.chaturv...@citrix.com> > *Subject: *Re: Review Request 20099: Domain-Account-User Sync Up Among > Multiple Regions (Core Changes) > > > > Alena, > > > > I understand that you have been helping a lot to make my codes to match > the coding standards, but I'm not sure what you mean by "the code base was > unnecessary huge". > > The initial implementation was to support the synchronization inside the > CS because this feature is missing in the current multiple region support, > and most of jobs were to separate the implementation from the CS because > you guys wanted me to provide it as a plugin. > > > > And I kept asking reviews for the design spec from when I published the > documents with initial prototype, it took a while for you to start to > review my implementation and they have been mostly about the coding > standards instead of the logic itself. So I'm saying that it would have > been better if there has been someone to review the design spec and the > prototype from the initial phase. > > > > Again, I really appreciate your help to come this far, but it was also > very painful for me. > > Thanks > > Alex Ough > > > > On Wed, Jun 25, 2014 at 10:41 PM, Alena Prokharchyk < > alena.prokharc...@citrix.com> wrote: > > Alex, > > > > In the beginning the code was not very well organazied, didn't match > coding standarts (no use of spring, misleading names, not segregated to its > own plugin), and the code base was unneccessary huge. > > All of the above it very hard to review and understand the code logic from > the beginning and engage more people to the review. Therefore Chiradeep > pointed it in his original review that the code needs to match CS standarts > first, and be better organized. I helped to review the fixes, and did logic > review as well after the code came into “reviewable” shape. > > > > I'm asking Kishan/Murali to look at it to see if anything is missing or > incorrect in the final review, not to make you override or change > everything you've already put in. > > > > Thank you, > > Alena. > > > > *From: *Alex Ough <alex.o...@sungardas.com> > *Date: *Wednesday, June 25, 2014 at 7:12 PM > > > *To: *Alena Prokharchyk <alena.prokharc...@citrix.com> > *Cc: *Kishan Kavala <kishan.kav...@citrix.com>, "dev@cloudstack.apache.org" > <dev@cloudstack.apache.org>, Murali Reddy <murali.re...@citrix.com>, Ram > Ganesh <ram.gan...@citrix.com>, Animesh Chaturvedi < > animesh.chaturv...@citrix.com> > *Subject: *Re: Review Request 20099: Domain-Account-User Sync Up Among > Multiple Regions (Core Changes) > > > > Alena, > > > > Don't get me wrong. What I'm saying is that it would have been better if > you asked the review to whomever you thought was important when you started > the review. > > > > Thanks > > Alex Ough > > > > On Wed, Jun 25, 2014 at 9:45 PM, Alena Prokharchyk < > alena.prokharc...@citrix.com> wrote: > > Alex, > > > > I did my best to review the code, made sure it came in shape with the CS > guidelines and java code style There was no way to anticipate all the > things to fix originally, as every subsequent review update added more > things to fix as the review code was new/refactored. > > > > And I don’t see anything wrong about asking for a FINAL opinion from other > people on the mailing list, considering some of them participated in the > review process along the way already (Kishan). Anybody can review the > review ticket till its closed, and point to the items that other reviewers > might have missed. > > > > Thank you, > > Alena. > > > > *From: *Alex Ough <alex.o...@sungardas.com> > *Date: *Wednesday, June 25, 2014 at 6:33 PM > *To: *Alena Prokharchyk <alena.prokharc...@citrix.com> > *Cc: *Kishan Kavala <kishan.kav...@citrix.com>, "dev@cloudstack.apache.org" > <dev@cloudstack.apache.org>, Murali Reddy <murali.re...@citrix.com>, Ram > Ganesh <ram.gan...@citrix.com>, Animesh Chaturvedi < > animesh.chaturv...@citrix.com> > > > *Subject: *Re: Review Request 20099: Domain-Account-User Sync Up Among > Multiple Regions (Core Changes) > > > > Thanks Alena, and I'm glad if they spend time for the review, but could it > be a little earlier for you to ask them to review instead of at the last > moment? > > I'm really exhausted with repeatedly added items whenever I post a review. > > > > Thanks > > Alex Ough > > > > On Wed, Jun 25, 2014 at 7:44 PM, Alena Prokharchyk < > alena.prokharc...@citrix.com> wrote: > > Alex, looks fine to me. Make sure that you put the regionId validation as > our in-built API validation won’t work in this case because there is no > UUID field support for the Region object. You can check how validation is > begin done in updateRegion/deleteRegion scenarios. > > > > Kishan/Murali, can you please spend some time doing the final review for > Alex’s tickets? As you are the original developers for Region, and probably > have the most expertise on the topic. I don’t want to commit the fixes > before I hear “ship it” from both of you, guys. > > > > Thanks, > > Alena. > > *From: *Alex Ough <alex.o...@sungardas.com> > *Date: *Wednesday, June 25, 2014 at 4:02 PM > *To: *Kishan Kavala <kishan.kav...@citrix.com> > *Cc: *Alena Prokharchyk <alena.prokharc...@citrix.com>, " > dev@cloudstack.apache.org" <dev@cloudstack.apache.org>, Murali Reddy < > murali.re...@citrix.com>, Ram Ganesh <ram.gan...@citrix.com>, Animesh > Chaturvedi <animesh.chaturv...@citrix.com> > > > *Subject: *Re: Review Request 20099: Domain-Account-User Sync Up Among > Multiple Regions (Core Changes) > > > > Hi Alena, > > > > Can you confirm if this fix is correct? > > > > @Parameter(name = ApiConstants.ORIGINATED_REGION_ID, type = > CommandType.INTEGER, description = "Region where this account is created.", > since = "4.5") > > private Integer originatedRegionId; > > > > Thanks > > Alex Ough > > > > On Wed, Jun 25, 2014 at 11:03 AM, Kishan Kavala <kishan.kav...@citrix.com> > wrote: > > Alex, > > You can refer to the code from initDataSource method in Transaction.java. > > > Properties file can be loaded using the following: > > > > *File dbPropsFile = PropertiesUtil.findConfigFile(propsFileName);* > > > > *From:* Alex Ough [mailto:alex.o...@sungardas.com] > *Sent:* Wednesday, 25 June 2014 4:31 PM > *To:* Kishan Kavala > *Cc:* Alena Prokharchyk; dev@cloudstack.apache.org; Murali Reddy; Ram > Ganesh; Animesh Chaturvedi > > > *Subject:* Re: Review Request 20099: Domain-Account-User Sync Up Among > Multiple Regions (Core Changes) > > > > Thanks Kishan, but there seems to be lots of 'db.properties' files, so > which one should be referenced? > > > > Alex Ough > > > > On Wed, Jun 25, 2014 at 2:25 AM, Kishan Kavala <kishan.kav...@citrix.com> > wrote: > > Alex, > > As Alena mentioned, it is admin’s responsibility to keep ids same across > Regions. Ids should be used as unique identifier. Region name is merely > descriptive name and its mostly associated with geographic location. > > Also note that region name can be updated anytime using updateRegion API. > > > > Unlike, other internal Ids in CS, region Ids are assigned by admin. So > exposing region Id to admin should not be an issue. > > > > Id of the local region cannot be guaranteed to be “1” always. Region Id > has to be unique across all regions. While creating new region admin will > provide unique region id to *cloud-setup-databases* script. Id of the > local region is stored in db.properties. To identify a Local region you can > use one of the following options: > > 1. Look up region.id in db.properties > > 2. Add a new column in region table > > > > > > *From:* Alex Ough [mailto:alex.o...@sungardas.com] > *Sent:* Wednesday, 25 June 2014 8:18 AM > *To:* Alena Prokharchyk > *Cc:* dev@cloudstack.apache.org; Kishan Kavala; Murali Reddy; Ram Ganesh; > Animesh Chaturvedi > > > *Subject:* Re: Review Request 20099: Domain-Account-User Sync Up Among > Multiple Regions (Core Changes) > > > > There is one thing that was not mentioned, which is that currently the id > of 'Local' region is always 1 and if we do not guarantee that, there is no > way to find out which is the local region unless we add one more field to > tells which is the local region. > > I'm wondering if we have a solution for this now. > > > > Thanks > > Alex Ough > > > > On Tue, Jun 24, 2014 at 9:59 PM, Alex Ough <alex.o...@sungardas.com> > wrote: > > I agree with that the ids are unique identifier, but they are usually > internal purpose not exposed to the users. So it is a little strange to ask > users to assign ids when they add new regions. And if we do not allow > duplicated names, I'm not sure why it is not good to use names as a unique > identifier. > > > > It's been a long way to come this far with several reasons, so I really > want to wrap this up as soon as possible, and this doesn't seem to be a > major obstacle, so let me just use 'id' as a parameter if there is no one > with a different thought until tomorrow morning. > > > > Thanks > > Alex Ough > > > > On Tue, Jun 24, 2014 at 8:52 PM, Alena Prokharchyk < > alena.prokharc...@citrix.com> wrote: > > Alex, id is used as a unique identifier for CS objects. And it is the CS > requirement to refer to the object by id if the id is present. Look at all > the other APIs. We nowhere refer to the network/vpc/vm by name just because > its more human readable. The id is used by Api layer when parameter > validation is done, by lots of Dao methods (findById is one of them), etc. > Even look at updateRegion/deleteRegion – we don’t refer to them by name, > but by the id. > > > > The reason why Kishan added the support for controlling the id by adding > it to the createRegion call (and making it unique) is exactly that – region > administrator can decide what id to set on the region, and to introduce the > region with the same id to the other regions’ db. > > > > So I would still suggest on using the id of the region in the API calls > you are modifying. Unless developers who worked on regions feature – > Kishan/Murali – come up with the valid objection. > > > > Thanks, > > Alena. > > > > *From: *Alex Ough <alex.o...@sungardas.com> > *Date: *Tuesday, June 24, 2014 at 5:41 PM > > > *To: *Alena Prokharchyk <alena.prokharc...@citrix.com> > *Cc: *"dev@cloudstack.apache.org" <dev@cloudstack.apache.org>, Kishan > Kavala <kishan.kav...@citrix.com>, Murali Reddy <murali.re...@citrix.com>, > Ram Ganesh <ram.gan...@citrix.com>, Animesh Chaturvedi < > animesh.chaturv...@citrix.com> > *Subject: *Re: Review Request 20099: Domain-Account-User Sync Up Among > Multiple Regions (Core Changes) > > > > We can use the same ids & names, but we don't have to use the same ids if > we use names, which is a little easier because names are user readable but > ids are not, so we don't need to memorize/check all the ids when we add new > regions in multiple regions, which can be confusing. > > > > Thanks > > Alex Ough > > > > On Tue, Jun 24, 2014 at 8:33 PM, Alena Prokharchyk < > alena.prokharc...@citrix.com> wrote: > > Aren’t we supposed to sync the regions across the multiple regions Dbs? > Because that’s what region FS states: > > > > > https://cwiki.apache.org/confluence/display/CLOUDSTACK/AWS-Style+Regions+Functional+Spec, > “Adding 2nd region” paragraph, bullet #4: > > > > 1. Install a 2nd CS instance. > > 2. While installing database set region_id using -r option in > cloud-setup-databases script (Make sure *database_key* is same across all > regions). > > *cloud-setup-databases cloud:**<**dbpassword**>**@localhost > --deploy-as=root:**<**password**>** -e **<**encryption_type**>** -m **<* > *management_server_key**>** -k **<**database_key**> -r <region_id>* > > 3. Start mgmt server > > 4. * Using addRegion API, add region 1 to region 2 and also region 2 to > region 1.* > > > > I assume that we expect the admin to add the region with the same name and > the same id to ALL regions Dbs (both id and name should be passed to > createRegion call). So they are all in sync. Isn’t it the requirement? If > so, we should rely on the fact that all regions Dbs will have the same set > of regions having the same ids and names cross regions. > > > > Thanks, > > Alena. > > *From: *Alex Ough <alex.o...@sungardas.com> > *Date: *Tuesday, June 24, 2014 at 5:17 PM > *To: *Alena Prokharchyk <alena.prokharc...@citrix.com> > *Cc: *"dev@cloudstack.apache.org" <dev@cloudstack.apache.org>, Kishan > Kavala <kishan.kav...@citrix.com>, Murali Reddy <murali.re...@citrix.com>, > Ram Ganesh <ram.gan...@citrix.com>, Animesh Chaturvedi < > animesh.chaturv...@citrix.com> > > > *Subject: *Re: Review Request 20099: Domain-Account-User Sync Up Among > Multiple Regions (Core Changes) > > > > What I'm trying to say is that when we pass the ids of regions, the > receivers do not know what the originated region is and the id of each > region is not same across all the regions. > > > > Thanks > > Alex Ough > > > > On Tue, Jun 24, 2014 at 7:35 PM, Alena Prokharchyk < > alena.prokharc...@citrix.com> wrote: > > Alex, thank you for summarizing. > > > > I still don’t see why id can’t be unique across regions as you can > control the id assignment – id is required when createRegion call is made. > And that’s how the region should be represented in other region’s Dbs – by > its id that is unique across the regions. Kishan/Murali, please confirm. > > > > Thank you, > > Alena. > > > > *From: *Alex Ough <alex.o...@sungardas.com> > *Date: *Tuesday, June 24, 2014 at 4:22 PM > *To: *"dev@cloudstack.apache.org" <dev@cloudstack.apache.org> > *Cc: *Alena Prokharchyk <alena.prokharc...@citrix.com>, Kishan Kavala < > kishan.kav...@citrix.com>, Murali Reddy <murali.re...@citrix.com>, Ram > Ganesh <ram.gan...@citrix.com>, Animesh Chaturvedi < > animesh.chaturv...@citrix.com> > > > *Subject: *Re: Review Request 20099: Domain-Account-User Sync Up Among > Multiple Regions (Core Changes) > > > > All, > > > > There is one open question in this topic, which is to figure out which > value is appropriate to pass the region object, id or name? > > During this implementation, we decided to add the information of regions > where user/account/domain objects have been originally > created/modified/removed. > > But the ids of regions are not same across the regions and currently the > regions do not have uuids(they will not be same either if we add them to > regions), so I'd like to use names. > > > > Please let me know what you think. > > Thanks > > Alex Ough > > > > On Tue, Jun 24, 2014 at 7:05 PM, Animesh Chaturvedi < > animesh.chaturv...@citrix.com> wrote: > > Let’s have the discussion on dev mailing list > > > > Thanks > > Animesh > > > > *From:* Alena Prokharchyk > *Sent:* Tuesday, June 24, 2014 3:06 PM > *To:* Alex Ough; Kishan Kavala; Murali Reddy > *Cc:* Animesh Chaturvedi; Ram Ganesh > > > *Subject:* Re: Review Request 20099: Domain-Account-User Sync Up Among > Multiple Regions (Core Changes) > > > > Adding Kishan to the thread as he was the one who implemented the region > feature originally. > > > > Kishan, in a situation when there are 2 regions in the system, we expect > “region” table to be populated with the same id/name in both Dbs for both > regions, right? So my question is – what uniquely identifies the region in > CS system in cross region setup – id/name? > > > > That unique identifier should be the value that is passed to the calls you > modify, Alex. WE can’t just pass some random name to the call without > making any further verification. > > > > Kishan/Murali, please help to verify this part of Alex’s fix as it should > really be someone with an expertise in Regions. I’ve reviewed the rest of > the feature, just this one item is open. See my latest comment to the > https://reviews.apache.org/r/17790/diff/?page=1#0 as well as refer to > this email thread for the context. > > > > -Alena. > > > > *From: *Alena Prokharchyk <alena.prokharc...@citrix.com> > *Date: *Tuesday, June 24, 2014 at 2:54 PM > *To: *Alex Ough <alex.o...@sungardas.com> > *Subject: *Re: Review Request 20099: Domain-Account-User Sync Up Among > Multiple Regions (Core Changes) > > > > That what would everybody assume 100% just by looking at the parameter > description and parameter – that you refer to region UUID : "Region where > this account is created.”/ORIGINATEDREGIONUUID > > In CS the UUID has a special meaning. It has to have the UUID format, and > its randomly generated value that is stored in the DB along with the actual > db id. I can see that regionVO lacks UUID field. Looks like existing > RegionVO object lacks this filed unlike other CS objects (uservm, etc). I > will follow up with Murali on that. > > > > Alex, so originatedRegionUUID refers to the region name, correct?. Why > don’t use the region id instead? That’s what we do when refer to CS objects > – we always refer to them by id which is unique. Which is true even for the > region: > > > > mysql> show create table region; > > > > UNIQUE KEY `id` (`id`), > > UNIQUE KEY `name` (`name`) > > > > > > That’s what you do when you manipulate the region itself > (delete/updateRegion) - refer to the region by its id. And this field is > returned to you when you call listRegions API: > > > > http://localhost:8096/?command=listRegions > > <region> > > <id>1</id> > > <name>Local</name> > > <endpoint>http://localhost:8080/client/</endpoint> > > <gslbserviceenabled>true</gslbserviceenabled> > > <portableipserviceenabled>false</portableipserviceenabled> > > </region> > > > > > > Please correct if I miss something. > > -Alena. > > > > *From: *Alex Ough <alex.o...@sungardas.com> > *Date: *Tuesday, June 24, 2014 at 2:33 PM > *To: *Alena Prokharchyk <alena.prokharc...@citrix.com> > *Subject: *Re: Review Request 20099: Domain-Account-User Sync Up Among > Multiple Regions (Core Changes) > > > > Thanks for the clarification, but here is a thing. > > > > I'm passing names as the values of originatedRegionUuids because the uuids > are randomly generated and the same regions do NOT have the same uuidss. > > So I'd like to change the parameter types into String. > > Let me know if you think otherwise. > > > > Thanks > > Alex Ough > > > > On Tue, Jun 24, 2014 at 5:17 PM, Alena Prokharchyk < > alena.prokharc...@citrix.com> wrote: > > Alex, > > > > take a look at ParamProcessWorker class, and how API parameters are being > dispatched/verified. > > > > > > 1) public void processParameters(final BaseCmd cmd, final Map params) > method > > > > First of all, EntityType parameter should be defined in the @Parameter > annotation for the originatedRegionID field. This parameter is used by > paramProcessWorker to make "if entity exists" validation > > > > > > 2) Check another method in the same class: > > > > private void setFieldValue(final Field field, final BaseCmd cmdObj, final > Object paramObj, final Parameter annotation) throws > IllegalArgumentException, ParseException { > > > > Thats the method responsible for dispatching/setting the field values. > Here is the snippet of the code for the case when UUID is defined: > > > > case UUID: > > if (paramObj.toString().isEmpty()) > > break; > > final Long internalId = > translateUuidToInternalId(paramObj.toString(), annotation); > > field.set(cmdObj, internalId); > > break; > > > > it always transforms the UUID to Long id, not string. And at the end, it > will be internal DB UUID, not the UUID. If you need the UUID, you have to > get it by a) retrieving the object from the DB by id b) Getting its UUID > property. > > > > If you leave it as a String, you will hit IllegalArgumentException at > "field.set(cmdObj, internalId);" line. > > > > > > Hope it answers your questions, and let me know if anything else needs to > be clarified. > > > > -alena. > > > > *From: *Alex Ough <alex.o...@sungardas.com> > *Date: *Tuesday, June 24, 2014 at 1:57 PM > > > *To: *Alena Prokharchyk <alena.prokharc...@citrix.com> > *Subject: *Re: Review Request 20099: Domain-Account-User Sync Up Among > Multiple Regions (Core Changes) > > > > Why do you want to change UUID to 'Long'? > > Can you just correct what I fixed? > > > > On Tue, Jun 24, 2014 at 4:21 PM, Alena Prokharchyk < > alena.prokharc...@citrix.com> wrote: > > You need to put: > > > > * the entityType parameter to the annotation. > > - Change the type to Long as I’ve already mentioned. Check how other > commands handle the parameters (networkId, vpcId, etc) > > —Alena. > > > > *From: *Alex Ough <alex.o...@sungardas.com> > *Date: *Tuesday, June 24, 2014 at 12:47 PM > > > *To: *Alena Prokharchyk <alena.prokharc...@citrix.com> > *Subject: *Re: Review Request 20099: Domain-Account-User Sync Up Among > Multiple Regions (Core Changes) > > > > Will this change work? > > > > @Parameter(name = ApiConstants.ORIGINATED_REGION_ID, type = > CommandType.UUID, description = "Region UUID where this account is > created.", since = "4.5") > > private String originatedRegionUUID; > > > > > > On Tue, Jun 24, 2014 at 3:25 PM, Alex Ough <alex.o...@sungardas.com> > wrote: > > Alena, > > > > This is what really frustrates me, but can you give the final items > instead of keeping adding more items whenever I post a review, please? > > Can you gurantee that this is the only item you want me to fix? > > > > On Tue, Jun 24, 2014 at 3:04 PM, Alena Prokharchyk < > alena.prokharc...@citrix.com> wrote: > > Alex, as a part of the fix, also change the param name to be regionId > (there should be a value in apiconstants already) as the parameter really > reflects CS region object, and we usually refer to those as networkID, > vpcID (not uuid) although uuid are passed in. Check if the rest of the api > changes you've done, respect this rule. Sorry, out of the office now and > cant check myself if there are any. > > -alena > > > > On Jun 24, 2014, at 11:12 AM, "Alena Prokharchyk" < > alena.prokharc...@citrix.com> wrote: > > > > > > ----------------------------------------------------------- > > > This is an automatically generated e-mail. To reply, visit: > > > https://reviews.apache.org/r/20099/#review46557 > > ----------------------------------------------------------- > > > > > > > Alex, one small thing. > > > > Just noticed that in the API commands you pass regionUUID as a string. > You should pass it as a type of UUID and specify the entityType parameter > in @Parameter so the entity validation is done correctly. Example: > > > > @Parameter(name=ApiConstants.ZONE_ID, type=CommandType.UUID, entityType > = ZoneResponse.class, > > required=true, description="the Zone ID for the network") > > private Long zoneId; > > > > That is the rule when passing id/uuid of the first class CS object to > the API call > > > > Then be aware of the fact that the APIDispatcher will transform UUID to > the actual DB id, and that would be the Id that you pass to the services > call. If what you need is UUID, not the actual id, to be saved in the > callContext, you have to transform it explicitly. > > > > - Alena Prokharchyk > > > > > > >> On June 24, 2014, 3:54 p.m., Alex Ough wrote: > >> > >> ----------------------------------------------------------- > > >> This is an automatically generated e-mail. To reply, visit: > >> https://reviews.apache.org/r/20099/ > > >> ----------------------------------------------------------- > >> > >> (Updated June 24, 2014, 3:54 p.m.) > >> > >> > >> Review request for cloudstack. > >> > >> > >> Repository: cloudstack-git > >> > >> > >> Description > >> ------- > > >> > >> This is the review request for the core changes related with #17790 > that has only the new plugin codes. > >> > >> > > >> Diffs > >> ----- > >> > >> api/src/com/cloud/event/EventTypes.java 0fa3cd5 > > >> api/src/com/cloud/user/AccountService.java eac8a76 > >> api/src/com/cloud/user/DomainService.java 4c1f93d > >> api/src/org/apache/cloudstack/api/ApiConstants.java adda5f4 > >> api/src/org/apache/cloudstack/api/BaseCmd.java ac9a208 > >> > api/src/org/apache/cloudstack/api/command/admin/account/CreateAccountCmd.java > 50d67d9 > >> > api/src/org/apache/cloudstack/api/command/admin/account/DeleteAccountCmd.java > 5754ec5 > >> > > api/src/org/apache/cloudstack/api/command/admin/account/DisableAccountCmd.java > 3e5e1d3 > >> > api/src/org/apache/cloudstack/api/command/admin/account/EnableAccountCmd.java > f30c985 > >> > api/src/org/apache/cloudstack/api/command/admin/account/LockAccountCmd.java > 3c185e4 > >> > api/src/org/apache/cloudstack/api/command/admin/account/UpdateAccountCmd.java > a7ce74a > >> > api/src/org/apache/cloudstack/api/command/admin/domain/CreateDomainCmd.java > 312c9ee > >> > api/src/org/apache/cloudstack/api/command/admin/domain/DeleteDomainCmd.java > a6d2b0b > >> > api/src/org/apache/cloudstack/api/command/admin/domain/UpdateDomainCmd.java > 409a84d > >> > api/src/org/apache/cloudstack/api/command/admin/region/AddRegionCmd.java > f6743ba > >> > api/src/org/apache/cloudstack/api/command/admin/region/UpdateRegionCmd.java > b08cbbb > >> > api/src/org/apache/cloudstack/api/command/admin/user/CreateUserCmd.java > 8f223ac > >> > api/src/org/apache/cloudstack/api/command/admin/user/DeleteUserCmd.java > 08ba521 > >> > api/src/org/apache/cloudstack/api/command/admin/user/DisableUserCmd.java > c6e09ef > >> > api/src/org/apache/cloudstack/api/command/admin/user/EnableUserCmd.java > d69eccf > >> api/src/org/apache/cloudstack/api/command/admin/user/LockUserCmd.java > 69623d0 > >> api/src/org/apache/cloudstack/api/command/admin/user/RegisterCmd.java > 2090d21 > >> > api/src/org/apache/cloudstack/api/command/admin/user/UpdateUserCmd.java > f21e264 > >> api/src/org/apache/cloudstack/api/response/RegionResponse.java 6c74fa6 > >> api/src/org/apache/cloudstack/region/Region.java df64e44 > >> api/src/org/apache/cloudstack/region/RegionService.java afefcc7 > >> api/test/org/apache/cloudstack/api/command/test/RegionCmdTest.java > 10c3d85 > >> client/pom.xml 29fef4f > >> > > engine/schema/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml > 2ef0d20 > >> engine/schema/src/com/cloud/user/AccountVO.java 0f5a044 > >> engine/schema/src/org/apache/cloudstack/region/RegionVO.java 608bd2b > >> > > plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/management/MockAccountManager.java > 4136b5c > >> plugins/pom.xml b5e6a61 > >> > > plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapCreateAccountCmd.java > b753952 > >> > > plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapImportUsersCmd.java > 6f7be90 > > >> server/src/com/cloud/api/ApiResponseHelper.java f1f0d2c > >> server/src/com/cloud/api/dispatch/ParamProcessWorker.java 1592b93 > >> server/src/com/cloud/event/ActionEventUtils.java 2b3cfea > >> server/src/com/cloud/projects/ProjectManagerImpl.java d10c059 > >> server/src/com/cloud/user/AccountManager.java 194c5d2 > >> server/src/com/cloud/user/AccountManagerImpl.java 7a889f1 > >> server/src/com/cloud/user/DomainManager.java f72b18a > >> server/src/com/cloud/user/DomainManagerImpl.java fbbe0c2 > >> server/src/org/apache/cloudstack/region/RegionManager.java 6f25481 > >> server/src/org/apache/cloudstack/region/RegionManagerImpl.java 8910714 > >> server/src/org/apache/cloudstack/region/RegionServiceImpl.java 98cf500 > >> server/test/com/cloud/user/AccountManagerImplTest.java 176cf1d > >> server/test/com/cloud/user/MockAccountManagerImpl.java 746fa1b > >> server/test/com/cloud/user/MockDomainManagerImpl.java 7dddefb > >> server/test/org/apache/cloudstack/region/RegionManagerTest.java d7bc537 > >> setup/db/db/schema-440to450.sql ee419a2 > >> ui/scripts/regions.js 368c1bf > >> > >> Diff: https://reviews.apache.org/r/20099/diff/ > >> > >> > >> Testing > >> ------- > >> > > >> 1. Successfully tested real time synchronization as soon as resources > are created/deleted/modified in one region. > >> 2. Successfully tested full scans to synchronize resources that were > missed during real time synchronization because of any reasons like network > connection issues. > >> 3. The tests were done manually and also automatically by randomly > generating changes each region. > >> > >> > > >> Thanks, > >> > >> Alex Ough > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
