On 6/16/14, 4:13 PM, "Alex Ough" <alex.o...@sungardas.com> wrote:
>On Mon, Jun 16, 2014 at 7:07 PM, Alena Prokharchyk < >alena.prokharc...@citrix.com> wrote: > >> >> ----------------------------------------------------------- >> This is an automatically generated e-mail. To reply, visit: >> https://reviews.apache.org/r/20099/#review45853 >> ----------------------------------------------------------- >> >> >> Alex, >> >> * add "since=version" attribute to all new parameters you've added to >>the >> existing API calls >> * remove listSyncAccounts/ >> listSyncUsers/listSyncDomains/listSyncDomainChildren >> > WHY?????? Because when the API commands come as a part of your plugin, they don¹t have to be registered in the commands.properties file. This file defines the permissions only, and you can define the permissions in your commands itself. Use ³authorized² parameter in the @ApiCommand annotation. Example: authorized = {RoleType.Admin} If the command is available to Root admin only (equivalent to =1 in commands.properties) > > >> * The important one - while on the call with Alex Huang, and later in >>all >> the countless emails, we've agreed not to modify the event itself, in >>order >> not to overload it with the details just specific to your plugin or the >> region. Now I see this in ActionEventUtils: >> >> eventDescription.put("oldentityname", oldEntityName); >> eventDescription.put("originatedregionuuid", originatedRegionUuid); >> > WHY??????? Alex, because in a numerous number of email threads below we¹ve agreed on the fact that the event object itself won¹t be overloaded and we will keep it simple. If every plugin overloads the event with the details only related to this plugin, there will be a mess. We¹ve discussed the way when your plugin modifies account/domain/user objects instead, and then reads from them. Then you said it wouldn¹t work and you would read from the CallContext (memory). You want me to pull out this thread? There was no notion about modifying/overloading the event object in the DB itself. If you can find it and put it here, I would appreciate that. > >> >> You said the parameter would be saved and read to/from CallContext only? >> Without touching anything else in the CS code like >> account/domain/user/event objects. Please remove it from the event. Or >>come >> up with a different solution that doesn't touch the event. >> > WHAT DO YOU MEAN?????? > >> >> - Alena Prokharchyk >> >> >> On June 15, 2014, 9:40 p.m., Alex Ough wrote: >> > >> > ----------------------------------------------------------- >> > This is an automatically generated e-mail. To reply, visit: >> > https://reviews.apache.org/r/20099/ >> > ----------------------------------------------------------- >> > >> > (Updated June 15, 2014, 9:40 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 39ef710 >> > api/src/com/cloud/user/AccountService.java 7e37b38 >> > api/src/com/cloud/user/DomainService.java 4c1f93d >> > api/src/org/apache/cloudstack/api/ApiConstants.java fdb4558 >> > api/src/org/apache/cloudstack/api/BaseCmd.java f6f21ae >> > >> >>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.ja >>va >> 3c185e4 >> > >> >>api/src/org/apache/cloudstack/api/command/admin/account/UpdateAccountCmd. >>java >> a7ce74a >> > >> >>api/src/org/apache/cloudstack/api/command/admin/domain/CreateDomainCmd.ja >>va >> 312c9ee >> > >> >>api/src/org/apache/cloudstack/api/command/admin/domain/DeleteDomainCmd.ja >>va >> a6d2b0b >> > >> >>api/src/org/apache/cloudstack/api/command/admin/domain/UpdateDomainCmd.ja >>va >> 409a84d >> > >> api/src/org/apache/cloudstack/api/command/admin/region/AddRegionCmd.java >> f6743ba >> > >> >>api/src/org/apache/cloudstack/api/command/admin/region/UpdateRegionCmd.ja >>va >> b08cbbb >> > >> api/src/org/apache/cloudstack/api/command/admin/user/CreateUserCmd.java >> 51e218d >> > >> 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 >> cf5d355 >> > 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 d8dbde7 >> > client/tomcatconf/commands.properties.in 45debe4 >> > >> >>engine/schema/resources/META-INF/cloudstack/core/spring-engine-schema-cor >>e-daos-context.xml >> 489b37d >> > 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/netw >>ork/contrail/management/MockAccountManager.java >> 957f708 >> > plugins/pom.xml 9b391b8 >> > >> >>plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/Ld >>apCreateAccountCmd.java >> 626bb8f >> > >> >>plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/Ld >>apImportUsersCmd.java >> 887ad00 >> > >> >>server/resources/META-INF/cloudstack/core/spring-server-core-managers-con >>text.xml >> fc1c7e2 >> > server/src/com/cloud/api/ApiResponseHelper.java 38f2f0b >> > server/src/com/cloud/api/dispatch/ParamProcessWorker.java e9bdd8b >> > server/src/com/cloud/event/ActionEventUtils.java 28e5680 >> > server/src/com/cloud/projects/ProjectManagerImpl.java d10c059 >> > server/src/com/cloud/user/AccountManager.java 03bf842 >> > server/src/com/cloud/user/AccountManagerImpl.java 2070ee6 >> > 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/MockAccountManagerImpl.java f373cba >> > server/test/com/cloud/user/MockDomainManagerImpl.java 7dddefb >> > server/test/org/apache/cloudstack/region/RegionManagerTest.java >>d7bc537 >> > setup/db/db/schema-440to450.sql 2bd5386 >> > ui/scripts/regions.js 66dae8c >> > >> > 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 >> > >> > >> >>
