Nice work Nirmal. +1 On Thu, Oct 9, 2014 at 10:28 AM, Lasindu Charith <[email protected]> wrote:
> Hi all, > > IMO, we should formalize the HTTP Return status codes throughout the REST > API. There are instances where the return status codes are misleading and > needs update. > > Thanks, > > On Mon, Oct 6, 2014 at 1:52 PM, Shiroshica Kulatilake <[email protected]> > wrote: > >> Hi, >> >> Added a few comments to the shared sheet. >> >> Mentioned below are some common comments which would apply to almost all; >> >> 1. Should we introduce DELETE for all (applicable) types of resources ? >> - Of course before doing a delete a series of checks will have to be done >> and maybe we can add a specific permission to delete operations so as to >> make sure it's secured >> 2. Response codes - We should always set the correct response code for >> each call based on the operation (we have done this in some places but not >> consistently) - this should also incorporate common server and client codes >> such as 403/ 401 - Have added a column in the shared doc - please validate >> 3. Making the REST API more user friendly >> 1. Making HEAD and OPTIONS available >> 2. Allowing HTTP method overriding >> 3. Introducing an error message (Mentioned oriinally by Akila) in a >> consumable format - e.g. error json with a id, message and description >> >> WDYT ? >> >> Thank you, >> Shiro >> >> Ref: >> 1. https://restful-api-design.readthedocs.org/en/latest/intro.html >> 2. >> http://www.vinaysahni.com/best-practices-for-a-pragmatic-restful-api#method-override >> >> On Mon, Oct 6, 2014 at 8:11 AM, Shiroshica Kulatilake <[email protected]> >> wrote: >> >>> >>> >>> On Mon, Oct 6, 2014 at 8:00 AM, Nirmal Fernando <[email protected]> >>> wrote: >>> >>>> I've enabled only comments since I am publishing it to dev list: >>>> https://docs.google.com/spreadsheets/d/1VP9-0xY2Oj1H1sv5yf5goIT6TakpwrCt5Z8QW1oFVOc/edit?usp=sharing >>>> >>> >>> Great will put in comments >>> >>> Thank you, >>> Shiro >>> >>> >>> >>>> On Mon, Oct 6, 2014 at 7:33 AM, Nirmal Fernando <[email protected] >>>> > wrote: >>>> >>>>> Sure, I hope, I'll be able to track who suggested what? >>>>> >>>>> On Mon, Oct 6, 2014 at 7:31 AM, Shiroshica Kulatilake <[email protected]> >>>>> wrote: >>>>> >>>>>> Hi Nirmal, >>>>>> >>>>>> Can you share this in the form of a sheet ? >>>>>> Then we can have a suggested column to have the diffs >>>>>> >>>>>> Thank you, >>>>>> Shiro >>>>>> >>>>>> On Mon, Oct 6, 2014 at 7:23 AM, Nirmal Fernando < >>>>>> [email protected]> wrote: >>>>>> >>>>>>> Thanks Akila for going through and identifying collisions. >>>>>>> >>>>>>> On Sun, Oct 5, 2014 at 11:26 PM, Akila Ravihansa Perera < >>>>>>> [email protected]> wrote: >>>>>>> >>>>>>>> Hi Nirmal, >>>>>>>> >>>>>>>> Great work on re-factoring REST API! >>>>>>>> >>>>>>>> I think these request paths might cause collisions. >>>>>>>> >>>>>>>> 1. /cartridges/multiTenant, /cartridges/singleTenant, >>>>>>>> /cartridges/{cartridgeType} >>>>>>>> >>>>>>> >>>>>>> I don't think in a real world, we would have a {cartridgeType} >>>>>>> called 'singleTenant' or 'multiTenant'. Do you think? >>>>>>> >>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> 2. /clusters/{cartridgeType}, /clusters/{subscriptionAlias}, >>>>>>>> /clusters/{clusterId} >>>>>>>> >>>>>>> >>>>>>> This is true, thanks again! So, we could refactor to something like: >>>>>>> >>>>>>> /clusters/cartridgeType/{cartridgeType} >>>>>>> /clusters/alias/{subscriptionAlias} >>>>>>> /clusters/{clusterId} >>>>>>> >>>>>>> Any other better way? >>>>>>> >>>>>>> >>>>>>>> >>>>>>>> I might be wrong...but just wanted to clarify :) >>>>>>>> >>>>>>>> Thanks. >>>>>>>> >>>>>>>> On Sun, Oct 5, 2014 at 9:06 PM, Udara Liyanage <[email protected]> >>>>>>>> wrote: >>>>>>>> >>>>>>>>> +1 for refactoring, some paths does not adhere to REST best >>>>>>>>> practices >>>>>>>>> >>>>>>>>> On Sun, Oct 5, 2014 at 8:54 PM, Nirmal Fernando < >>>>>>>>> [email protected]> wrote: >>>>>>>>> >>>>>>>>>> Hi Guys, >>>>>>>>>> >>>>>>>>>> As discussed previously, I've been going through all the API >>>>>>>>>> operations exposed via Stratos REST API and identified following set >>>>>>>>>> of >>>>>>>>>> APIs need to be refactored to have a RESTful design. While doing >>>>>>>>>> this, I've >>>>>>>>>> also identified sub-APIs that we have provided (under Entity column) >>>>>>>>>> and I >>>>>>>>>> suggest we could introduce sub-API classes as it makes the >>>>>>>>>> StratosAdmin >>>>>>>>>> class more readable. >>>>>>>>>> >>>>>>>>>> There're 40+ API operations ([?]), hence there could be places >>>>>>>>>> that I've not suggested proper refactoring. Please go through the >>>>>>>>>> list and >>>>>>>>>> provide improvements/comments, we can discuss/argue, then conclude >>>>>>>>>> and >>>>>>>>>> build a clean API for Stratos. >>>>>>>>>> >>>>>>>>>> IMO we also need to place a review process when adding a new API, >>>>>>>>>> it could save lot of time spending on cleaning things up. I propose, >>>>>>>>>> API >>>>>>>>>> introducer should initiate a discuss thread and a vote before >>>>>>>>>> committing >>>>>>>>>> them to the repository. >>>>>>>>>> >>>>>>>>>> I'll keep this thread open for 48hrs for comments and/or >>>>>>>>>> improvements. >>>>>>>>>> >>>>>>>>>> EntityHTTP OperationAPI Operation Name -> Suggested NameRequest >>>>>>>>>> PathProposed PathCartridgePOSTdeployCartridgeDefinition >>>>>>>>>> /cartridge/definition//cartridgesDELETE >>>>>>>>>> unDeployCartridgeDefinition/cartridge/definition/{cartridgeType} >>>>>>>>>> /cartridges/{cartridgeType}GETgetValidDeploymentPolicies -> >>>>>>>>>> getValidDeploymentPoliciesOfCartridge >>>>>>>>>> {cartridgeType}/policy/deployment >>>>>>>>>> /cartridges/{cartridgeType}/deploymentPolicyGETgetAvailableMultiTenantCartridges >>>>>>>>>> -> getMultiTenantCartridges/cartridge/tenanted/list >>>>>>>>>> /cartridges/multiTenantGETgetAvailableSingleTenantCartridges -> >>>>>>>>>> getSingleTenantCartridges/cartridge/list/cartridges/singleTenant >>>>>>>>>> GETgetAvailableCartridges -> getCartridges >>>>>>>>>> /cartridge/available/list/cartridgesGETgetAvailableSingleTenantCartridgeInfo >>>>>>>>>> -> getAvailableSingleTenantCartridge >>>>>>>>>> /cartridge/available/info/{cartridgeType} >>>>>>>>>> /cartridges/{cartridgeType}/singleTenantGET >>>>>>>>>> getAvailableLbCartridges/cartridge/lb/cartridges/loadBalancer >>>>>>>>>> PartitionPOSTdeployPartition/policy/deployment/partition >>>>>>>>>> /partitionsGETgetPartitions/partition/partitionsGETgetPartition >>>>>>>>>> /partition/{partitionId}/partitions/{partitionId}Autoscaling >>>>>>>>>> PolicyPOSTdeployAutoscalingPolicyDefintion/policy/autoscale >>>>>>>>>> /autoscalePoliciesGETgetAutoscalePolicies -> >>>>>>>>>> getAutoscalingPolicies/policy/autoscale/autoscalePoliciesGETgetAutoscalePolicies >>>>>>>>>> -> getAutoscalingPolicy/policy/autoscale/{autoscalePolicyId} >>>>>>>>>> /autoscalePolicies/{autoscalePolicyId}Deployment PolicyPOST >>>>>>>>>> deployDeploymentPolicyDefinition/policy/deployment >>>>>>>>>> /deploymentPoliciesGETgetDeploymentPolicies/policy/deployment >>>>>>>>>> /deploymentPoliciesGETgetDeploymentPolicies -> >>>>>>>>>> getDeploymentPolicy/policy/deployment/{deploymentPolicyId} >>>>>>>>>> /deploymentPolicies/{deploymentPolicyId}Partition GroupGET >>>>>>>>>> getPartitionGroups/partition/group/{deploymentPolicyId} >>>>>>>>>> /deploymentPolicies/{deploymentPolicyId}/partitionGroupGETgetPartitions >>>>>>>>>> -> getPartitionGroup >>>>>>>>>> /partition/{deploymentPolicyId}/{partitionGroupId} >>>>>>>>>> /deploymentPolicies/{deploymentPolicyId}/partitionGroup/{partitionGroupId} >>>>>>>>>> GETgetPartitionsOfPolicy/partition/{deploymentPolicyId} >>>>>>>>>> /deploymentPolicies/{deploymentPolicyId}/partitionSubscriptions >>>>>>>>>> GETgetSubscribedCartridges/cartridge/list/subscribed >>>>>>>>>> /subscriptions/cartridgesGETgetSubscribedCartridgesForServiceGroup >>>>>>>>>> -> getSubscribedCartridgesOfServiceGroup >>>>>>>>>> /cartridge/list/subscribed/group/{serviceGroup} >>>>>>>>>> /subscriptions/cartridges/groups/{serviceGroup}GETgetCartridgeInfo >>>>>>>>>> -> getSubscribedCartridgeInfo/cartridge/info/{subscriptionAlias} >>>>>>>>>> /subscriptions/{subscriptionAlias}/cartridgesGETgetActiveInstances >>>>>>>>>> -> getActiveMembersCountOfSubscription >>>>>>>>>> /cartridge/active/{cartridgeType}/{subscriptionAlias} >>>>>>>>>> /subscriptions/{subscriptionAlias}/cartridges/{cartridgeType}/active >>>>>>>>>> POSTsubscribe/cartridge/subscribe/subscriptionsDELETEunsubscribe >>>>>>>>>> /cartridge/unsubscribe/subscriptions/{subscriptionAlias}POST >>>>>>>>>> addSubscriptionDomains >>>>>>>>>> /cartridge/{cartridgeType}/subscription/{subscriptionAlias}/domains >>>>>>>>>> /subscriptions/{subscriptionAlias}/domainsGET >>>>>>>>>> getSubscriptionDomains >>>>>>>>>> /cartridge/{cartridgeType}/subscription/{subscriptionAlias}/domains >>>>>>>>>> /subscriptions/{subscriptionAlias}/domainsGET >>>>>>>>>> getSubscriptionDomain >>>>>>>>>> /cartridge/{cartridgeType}/subscription/{subscriptionAlias}/domains/{domainName} >>>>>>>>>> /subscriptions/{subscriptionAlias}/domains/{domainName}DELETE >>>>>>>>>> removeSubscriptionDomain >>>>>>>>>> /cartridge/{cartridgeType}/subscription/{subscriptionAlias}/domains/{domainName} >>>>>>>>>> /subscriptions/{subscriptionAlias}/domains/{domainName}Clusters >>>>>>>>>> GETgetClustersForTenant -> getClustersOfTenant/cluster//clusters >>>>>>>>>> GETgetClusters -> getClustersOfCartridge/cluster/{cartridgeType}/ >>>>>>>>>> /clusters/{cartridgeType}GETgetServiceClusters (This seems to do >>>>>>>>>> the same job as getClusters - I suggest we deprecate >>>>>>>>>> this.)GETgetCluster >>>>>>>>>> -> getClusterOfSubscription >>>>>>>>>> /cluster/{cartridgeType}/{subscriptionAlias} >>>>>>>>>> /clusters/{subscriptionAlias}GETgetCluster >>>>>>>>>> /cluster/clusterId/{clusterId}/clusters/{clusterId}GET >>>>>>>>>> getLoadBalancerCluster >>>>>>>>>> /cartridge/{cartridgeType}/subscription/{subscriptionAlias}/load-balancer-cluster >>>>>>>>>> /clusters/{subscriptionAlias}/loadBalancerTenantsGET >>>>>>>>>> retrieveTenants/tenant/list/tenants/list(All paths will be >>>>>>>>>> refactored to start from /tenants )Services POSTdeployService >>>>>>>>>> /service/definition/servicesDELETEunDeployService >>>>>>>>>> /service/definition/{serviceType}/services/{serviceType}(All >>>>>>>>>> paths will be refactored to start from /services )GIT >>>>>>>>>> RepositoriesPOSTgetRepoNotification -> notifyRepository >>>>>>>>>> /reponotification/repo/notifyPOSTsynchronizeRepository -> >>>>>>>>>> synchronizeRepositoryOfSubscription/cartridge/sync >>>>>>>>>> /repo/synchronize/{subscriptionAlias}UsersGETretrieveUsers >>>>>>>>>> /user/list/usersKubernetes HostsPOSTdeployKubernetesGroup -> >>>>>>>>>> deployKubernetesHostCluster/kubernetes/deploy/group >>>>>>>>>> /kubernetesClusterPUTdeployKubernetesHost >>>>>>>>>> /kubernetes/deploy/host/{kubernetesGroupId} >>>>>>>>>> /kubernetesCluster/{kubernetesClusterId}/minionPUT >>>>>>>>>> updateKubernetesMaster/kubernetes/update/master >>>>>>>>>> /kubernetesCluster/{kubernetesClusterId}/masterPATCH >>>>>>>>>> updateKubernetesHost/kubernetes/update/host >>>>>>>>>> /kubernetesCluster/{kubernetesClusterId}/minion/{minionId}GETgetKubernetesGroups >>>>>>>>>> -> getKubernetesHostClusters/kubernetes/group/kubernetesCluster >>>>>>>>>> GETgetKubernetesGroup -> getKubernetesHostCluster >>>>>>>>>> /kubernetes/group/{kubernetesGroupId} >>>>>>>>>> /kubernetesCluster/{kubernetesClusterId}GETgetKubernetesHosts -> >>>>>>>>>> getKubernetesHostsOfKubernetesCluster >>>>>>>>>> /kubernetes/hosts/{kubernetesGroupId} >>>>>>>>>> /kubernetesCluster/{kubernetesClusterId}/hostsGETgetKubernetesMaster >>>>>>>>>> -> getKubernetesMasterOfKubernetesCluster >>>>>>>>>> /kubernetes/master/{kubernetesGroupId} >>>>>>>>>> /kubernetesCluster/{kubernetesClusterId}/masterDELETEunDeployKubernetesGroup >>>>>>>>>> -> unDeployKubernetesHostCluster >>>>>>>>>> /kubernetes/group/{kubernetesGroupId} >>>>>>>>>> /kubernetesCluster/{kubernetesClusterId}DELETEunDeployKubernetesHost >>>>>>>>>> -> unDeployKubernetesHostOfKubernetesCluster >>>>>>>>>> /kubernetes/host/{kubernetesHostId} >>>>>>>>>> /kubernetesCluster/{kubernetesClusterId}/hosts/{hostId} >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> -- >>>>>>>>>> Best Regards, >>>>>>>>>> Nirmal >>>>>>>>>> >>>>>>>>>> Nirmal Fernando. >>>>>>>>>> PPMC Member & Committer of Apache Stratos, >>>>>>>>>> Senior Software Engineer, WSO2 Inc. >>>>>>>>>> >>>>>>>>>> Blog: http://nirmalfdo.blogspot.com/ >>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> -- >>>>>>>>> >>>>>>>>> Udara Liyanage >>>>>>>>> Software Engineer >>>>>>>>> WSO2, Inc.: http://wso2.com >>>>>>>>> lean. enterprise. middleware >>>>>>>>> >>>>>>>>> web: http://udaraliyanage.wordpress.com >>>>>>>>> phone: +94 71 443 6897 >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> -- >>>>>>>> Akila Ravihansa Perera >>>>>>>> Software Engineer, WSO2 >>>>>>>> >>>>>>>> Blog: http://ravihansa3000.blogspot.com >>>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> Best Regards, >>>>>>> Nirmal >>>>>>> >>>>>>> Nirmal Fernando. >>>>>>> PPMC Member & Committer of Apache Stratos, >>>>>>> Senior Software Engineer, WSO2 Inc. >>>>>>> >>>>>>> Blog: http://nirmalfdo.blogspot.com/ >>>>>>> >>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> Shiroshica Kulatilake >>>>>> >>>>>> Architect, >>>>>> WSO2, Inc. http://wso2.com/ >>>>>> Phone: +94 776523867 >>>>>> >>>>> >>>>> >>>>> >>>>> -- >>>>> Best Regards, >>>>> Nirmal >>>>> >>>>> Nirmal Fernando. >>>>> PPMC Member & Committer of Apache Stratos, >>>>> Senior Software Engineer, WSO2 Inc. >>>>> >>>>> Blog: http://nirmalfdo.blogspot.com/ >>>>> >>>> >>>> >>>> >>>> -- >>>> Best Regards, >>>> Nirmal >>>> >>>> Nirmal Fernando. >>>> PPMC Member & Committer of Apache Stratos, >>>> Senior Software Engineer, WSO2 Inc. >>>> >>>> Blog: http://nirmalfdo.blogspot.com/ >>>> >>> >>> >>> >>> -- >>> Shiroshica Kulatilake >>> >>> Architect, >>> WSO2, Inc. http://wso2.com/ >>> Phone: +94 776523867 >>> >> >> >> >> -- >> Shiroshica Kulatilake >> >> Architect, >> WSO2, Inc. http://wso2.com/ >> Phone: +94 776523867 >> > > > > -- > *Lasindu Charith* > Software Engineer, WSO2 Inc. > Mobile: +94714427192 > Web: blog.lasindu.com > -- Pradeep Fernando. http://pradeepfernando.blogspot.com/
