cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chiradeep Vittal <>
Subject Re: [MERGE] Regions branch to master
Date Tue, 29 Jan 2013 20:11:36 GMT
0. Doesn't compile (commit d3089ba2a5684)
1. Why does the createRegion API take an integer id? I think I know the
answer, but it could be documented in the FS / code
2. Why doesn't the createRegion API check for duplicate region names?
3. I'd argue that end users are more familiar with region names than ids.
4. Describe API and secret key in greater detail in the FS and annotation.
Why is it needed?
5. I think we should move away from implementing the service api and the
provisioning/orchestration api in the same file. So, split into
RegionServiceImpl and RegionManagerImpl
6. Do we really need propagate* APIs? (note spelling mistake in actual
API). This is going to be so error prone and full of corner cases that
will require manual fiddling to fix anyway. Can we assume a 3rd party
service that will perform the sync?
7. Please add more comments (schema, interface, api)
8. The new package naming scheme seems to be org.apache.cloudstack, so
could we use that?

On 1/29/13 6:47 AM, "Chip Childers" <> wrote:

>On Tue, Jan 29, 2013 at 4:45 AM, Kishan Kavala <>
>> I would like to merge Regions feature to master
>> Spec:
>> Jira ticket:
>> Branch:
>> regions (latest master code is merged to regions branch)
>> Notes:
>> 1. If an environment has only 1 region, functionality will be same as
>>the current CS installation, no impact.
>> 2. In multi-region setup, any failure in propagating
>>account/user/domain changes to other regions will be logged in
>>region_sync table. These changes have to be propagated manually.
>> 3. Branch is updated with latest master and all unit tests passed
>> Regards,
>> Kishan
>+1 from me!  We probably want someone else to comment as well, but I'm
>really glad to see that the tests were extended to account for the
>region entity.
>I also appreciate that the addition of the region ID param for the API
>calls remained optional.

View raw message