Hi Nirmal, Please find the comments inline..
On Wed, Nov 6, 2013 at 7:37 AM, Nirmal Fernando <[email protected]>wrote: > Hi Pradeep, > > Thanks for bringing this up. > > On Nov 4, 2013 10:02 PM, "Pradeep Fernando" <[email protected]> wrote: > > > > Hi devs, > > > > I recently worked on porting tenant mgt operations to REST API. Below is > the API i came up with. > > > > Main operations, > > > > - addTenant > > - deleteTenant > We need to carefully support this operation since upon this event, we need > to remove all existing subscriptions etc. > > > - updateTenant > We should not allow changing tenant domain IMO, but a set of selected > attributes. > > > - getTenant > > - search tenants by a String > > - activateTenant > > - deactivateTenant > > - getCompleteTenantList > Is there any reason why this is not 'getAllTenants'? All the above methods are wrapper methods. I did not came up with method names/new implementation flows. The operation names for taken from existing web service wrapper. Yes delete tenant is bit tricky. I noticed that, we are not using that operation in WS api, even if its there. Furthermore i encountered an error coming from carbon registry while executing the operation. However i ported the code for the completeness, sake. should i rename some of the method names still ? > > > > > > > > > API i came up with, > > > > Considered tenant as a resource, > > the below string is formatted as , operation, HTTPMethod, REST endpoint, > payload > > > > - addTenant , POST, /admin/tenant, tenantInfoBean as a JSON payload > > - deleteTenant, DELETE, /admin/tenant/{tenantDomain} > > - updateTenant, PUT, /admin/tenant, tenantInfoBean as a JSON payload > Don't we need tenant domain at the end? That would make it easy to > authorize? > > > - getTenant, GET, /admin/tenant/{tenantDomain} > > > > I'm bit confused on the admin context you have used here. For an example > to add a tenant you have to be in super tenant mode and to update a tenant > IMO you need to be in tenant admin mode. > > But I'm not sure whether that should exhibit in the url. > well, /admin context is the qualifying pre-context i used. Its solely for namespace purposes. (rationale was like, these are the endpoint for Stratos admin operations). i just wanted to prevent the URL endpoint being formatted as, <webappContext>/tenant etc. should we choose a different context/drop the context altogether ? Appreciate the feedback. thanks. --Pradeep > > The other operations are kind of utility operations. I could not > co-relate them to any resource. > > > > - search tenants by a String, GET, /admin/tenant/search/{searchString} > > - activateTenant , POST, /admin/tenant/activate/{tenantDomain} > > - deactivateTenant, POST, /admin/tenant/deactivate/{tenantDomain} > > - getCompleteTenantList, GET, /admin/tenant/list > > > > > > I used POST method in tenant activate/deactivate operations, because it > modifies the tenant aspect... > > IMHO we do not need versioning info in our REST API endpoints. There > will be one stratos controller for a given setup and I do not see a use > case for maintaining two different version of the same admin API.. > > > +1 > > > > > I have attached the patch for above implementation, to the JIRA [1] > > > > The above API is open for discussion and i can change the current impl > if required.. > > > > [1] https://issues.apache.org/jira/browse/STRATOS-152 > > > > > > > > On Fri, Oct 4, 2013 at 11:07 PM, Pradeep Fernando <[email protected]> > wrote: > >> > >> Hi All, > >> > >> This thread is to discuss about the admin services required by Stratos > Controller and its API definitions. > >> > >> your thoughts/suggestions are highly appreciated. > >> > >> thanks, > >> --Pradeep > > > > > > > > > > -- > > Pradeep Fernando. > > http://pradeepfernando.blogspot.com/ > > -- Pradeep Fernando. http://pradeepfernando.blogspot.com/
