Also, I don't see any new tests. The response object is supposed to be annotated with the table name.
On 1/29/13 12:11 PM, "Chiradeep Vittal" <chiradeep.vit...@citrix.com> wrote: >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" <chip.child...@sungard.com> wrote: > >>On Tue, Jan 29, 2013 at 4:45 AM, Kishan Kavala <kishan.kav...@citrix.com> >>wrote: >>> I would like to merge Regions feature to master >>> >>> Spec: >>> >>>https://cwiki.apache.org/confluence/display/CLOUDSTACK/AWS-Style+Regions >>>+ >>>Functional+Spec >>> >>> Jira ticket: >>> https://issues.apache.org/jira/browse/CLOUDSTACK-241 >>> >>> 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 >> >>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. >> >>Thanks! >
