+1 great work Nirmal, sorry for late reply I just saw this! I would like to add something over status code, what if we return 204 no-content status code when something successfully deleted.Further I believe if we can map proper HTTP status code when an error occurred would be nice for consumers, for an example.
when adding a new cartridge, if that particular cartridge is already exists, we can return 409 conflicts error code etc.. On Fri, Oct 10, 2014 at 3:14 AM, Pradeep Fernando <[email protected]> wrote: > 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/ > -- Roshan Wijesena. Senior Software Engineer-WSO2 Inc. Mobile: *+94752126789* Email: [email protected] *WSO2, Inc. :** wso2.com <http://wso2.com/>* lean.enterprise.middleware.
