-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17790/#review39701
-----------------------------------------------------------


Alex, can you please


1) split your fix into 2 patches:

Patch #1 – contains the fixes to CS core/api 
Patch #2 – fixes for your new plugin

Check if review board allows to upload 2 patches for the same ticket; if it 
doesn’t   - create a new one for the second patch.

2) Move all the Daos related to your plugin (RmapVO.java), to the plugin 
folder? As an example, you can take a look at NetScalerPodDao

3) There is no need to introduce your own exceptions in the plugin 
UnsupportedException.java/InvalidDataException.java/APIFailureException.java. 
You should re-use existing CS exceptions like InvalidParameterValueException 
(instead of InvalidDataException), 
UnsupportedException(UnsupportedServiceException), etc. Unless your exceptions 
are very different from those; then can you please explain the differences.


4) ActionEventUtils.java

String concatenation again :) Can you please replace all "+" operations in 
addDescription with StringBuffer?

- Alena Prokharchyk


On April 6, 2014, 3:32 a.m., Alex Ough wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17790/
> -----------------------------------------------------------
> 
> (Updated April 6, 2014, 3:32 a.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Currently, under the environment of cloudstack with multiple regions, each 
> region has its own management server running with a separate database, which 
> will cause data discrepancies when users create/update/delete 
> domain/account/user data independently in each management server. So to 
> support multiple regions and provide one point of entry for each customer, 
> this implementation duplicates domain/account/user information of customers 
> in one region to all of the regions independently whenever there is any 
> change.
> 
> https://issues.apache.org/jira/browse/CLOUDSTACK-4992
> https://cwiki.apache.org/confluence/display/CLOUDSTACK/Domain-Account-User+Sync+Up+Among+Multiple+Regions
> 
> 
> Diffs
> -----
> 
>   api/src/com/cloud/domain/Domain.java 365a705 
>   api/src/com/cloud/event/EventTypes.java 39ef710 
>   api/src/com/cloud/user/Account.java b912e51 
>   api/src/com/cloud/user/AccountService.java 7e37b38 
>   api/src/com/cloud/user/User.java 36e9028 
>   api/src/com/cloud/user/UserAccount.java c5a0637 
>   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/region/AddRegionCmd.java 
> f6743ba 
>   api/src/org/apache/cloudstack/api/command/admin/region/UpdateRegionCmd.java 
> b08cbbb 
>   api/src/org/apache/cloudstack/api/response/AccountResponse.java 2e50c51 
>   api/src/org/apache/cloudstack/api/response/DomainResponse.java 0c0281e 
>   api/src/org/apache/cloudstack/api/response/RegionResponse.java 6c74fa6 
>   api/src/org/apache/cloudstack/api/response/UserResponse.java 40e1561 
>   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 
>   
> engine/schema/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml
>  489b37d 
>   engine/schema/src/com/cloud/domain/DomainVO.java f6494b3 
>   engine/schema/src/com/cloud/rmap/RmapVO.java PRE-CREATION 
>   engine/schema/src/com/cloud/rmap/dao/RmapDao.java PRE-CREATION 
>   engine/schema/src/com/cloud/rmap/dao/RmapDaoImpl.java PRE-CREATION 
>   engine/schema/src/com/cloud/user/AccountVO.java 0f5a044 
>   engine/schema/src/com/cloud/user/UserAccountVO.java cef9239 
>   engine/schema/src/com/cloud/user/UserVO.java 68879f6 
>   engine/schema/src/org/apache/cloudstack/region/RegionVO.java 608bd2b 
>   framework/db/src/com/cloud/utils/db/Attribute.java 82c2bdb 
>   framework/db/src/com/cloud/utils/db/GenericDao.java cb401cd 
>   framework/db/src/com/cloud/utils/db/GenericDaoBase.java 2052aad 
>   framework/db/src/com/cloud/utils/db/SqlGenerator.java befe34b 
>   framework/db/test/com/cloud/utils/db/GenericDaoBaseTest.java aef0c69 
>   framework/db/test/com/cloud/utils/db/SqlGeneratorTest.java PRE-CREATION 
>   plugins/event-bus/multiregion/pom.xml PRE-CREATION 
>   
> plugins/event-bus/multiregion/resources/META-INF/cloudstack/spring-mom-multiregion-daos-context.xml
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/resources/META-INF/cloudstack/system/spring-plugin-multiregion-system-context.xml
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/FullSyncer.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/InjectedCollection.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/MultiRegionEventBus.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/StringManipulator.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/api/AccountCaller.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/api/BaseCaller.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/api/DomainCaller.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/api/UserCaller.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/exception/APIFailureException.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/exception/InvalidDataException.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/exception/UnsupportedException.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/service/AccountFullSyncProcessor.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/service/AccountService.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/service/BaseService.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/service/DomainFullSyncProcessor.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/service/DomainService.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/service/FullScanner.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/service/FullSyncProcessor.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/service/LocalAccountManager.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/service/LocalDomainManager.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/service/LocalUserManager.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/service/RemoteAccountEventProcessor.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/service/RemoteDomainEventProcessor.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/service/RemoteEventProcessor.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/service/RemoteUserEventProcessor.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/service/UserFullSyncProcessor.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/service/UserService.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/simulator/SimulatorAccountLocalGenerator.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/simulator/SimulatorAccountLocalGeneratorEvent.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/simulator/SimulatorAutoGenerator.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/simulator/SimulatorDomainLocalGenerator.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/simulator/SimulatorDomainLocalGeneratorEvent.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/simulator/SimulatorLocalGenerator.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/simulator/SimulatorUserLocalGenerator.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/simulator/SimulatorUserLocalGeneratorEvent.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/subscriber/AccountSubscriber.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/subscriber/DomainSubscriber.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/subscriber/MultiRegionSubscriber.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/src/org/apache/cloudstack/mom/multiregion/subscriber/UserSubscriber.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/test/org/apache/cloudstack/mom/multiregion/api/AccountCallerTest.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/test/org/apache/cloudstack/mom/multiregion/api/BaseCallerTest.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/test/org/apache/cloudstack/mom/multiregion/api/DomainCallerTest.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/test/org/apache/cloudstack/mom/multiregion/api/UserCallerTest.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/test/org/apache/cloudstack/mom/multiregion/service/AccountFullSyncProcessorTest.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/test/org/apache/cloudstack/mom/multiregion/service/BaseServiceTest.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/test/org/apache/cloudstack/mom/multiregion/service/DomainFullSyncProcessorTest.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/test/org/apache/cloudstack/mom/multiregion/service/RemoteAccountEventProcessorTest.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/test/org/apache/cloudstack/mom/multiregion/service/RemoteDomainEventProcessorTest.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/test/org/apache/cloudstack/mom/multiregion/service/RemoteUserEventProcessorTest.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/test/org/apache/cloudstack/mom/multiregion/service/UserFullSyncProcessorTest.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/test/org/apache/cloudstack/mom/multiregion/simulator/SimulatorAccountLocalGeneratorEventTest.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/test/org/apache/cloudstack/mom/multiregion/simulator/SimulatorAccountLocalGeneratorTest.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/test/org/apache/cloudstack/mom/multiregion/simulator/SimulatorDomainLocalGeneratorEventTest.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/test/org/apache/cloudstack/mom/multiregion/simulator/SimulatorDomainLocalGeneratorTest.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/test/org/apache/cloudstack/mom/multiregion/simulator/SimulatorLocalGeneratorTest.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/test/org/apache/cloudstack/mom/multiregion/simulator/SimulatorUserLocalGeneratorEventTest.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/test/org/apache/cloudstack/mom/multiregion/simulator/SimulatorUserLocalGeneratorTest.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/test/org/apache/cloudstack/mom/multiregion/subscriber/AccountSubscriberTest.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/test/org/apache/cloudstack/mom/multiregion/subscriber/DomainSubscriberTest.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/test/org/apache/cloudstack/mom/multiregion/subscriber/MultiRegionSubscriberTest.java
>  PRE-CREATION 
>   
> plugins/event-bus/multiregion/test/org/apache/cloudstack/mom/multiregion/subscriber/UserSubscriberTest.java
>  PRE-CREATION 
>   
> plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/management/MockAccountManager.java
>  957f708 
>   plugins/pom.xml 9b391b8 
>   
> server/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml
>  fc1c7e2 
>   server/src/com/cloud/api/ApiDispatcher.java 95074e2 
>   server/src/com/cloud/api/ApiResponseHelper.java 38f2f0b 
>   server/src/com/cloud/api/dispatch/ParamProcessWorker.java e9bdd8b 
>   server/src/com/cloud/api/query/dao/AccountJoinDaoImpl.java ecd97c7 
>   server/src/com/cloud/api/query/dao/UserAccountJoinDaoImpl.java 923a238 
>   server/src/com/cloud/api/query/vo/AccountJoinVO.java 8d642ed 
>   server/src/com/cloud/api/query/vo/UserAccountJoinVO.java ed29284 
>   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/api/dispatch/ParamProcessWorkerTest.java 12051a6 
>   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/17790/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
> 
>

Reply via email to