Daan, I don’t see the need for doing that. Take deleteAccountFromProject API. Every delete command in CloudStack returns the success response; seeing true/false is more than enough to figure out if the deletion was successful. We follow this format in every delete* command in CS.
Now take addAccountToProject. This command neither modifies the account, nor the project. It just creates a relationship between project and the account, that you can later get by executing listProjectAccounts API. Although we might have return success/accountObject-ProjectId in the response instead of just returning the success, I don’t see the big need for that. But if you insist on fixing it anyway, yes, the right way would be extending SuccessResponse and use it in this particular API. Although the response name “success” that we have to preserve, would be a bit misleading considering that we are gonna return other information besides just boolean param. -Alena. On 3/4/14, 10:48 AM, "Daan Hoogland" <daan.hoogl...@gmail.com> wrote: >I mean not returning a SuccessResponse but some custom variety to it. > >On Tue, Mar 4, 2014 at 7:44 PM, Alena Prokharchyk ><alena.prokharc...@citrix.com> wrote: >> We are not gonna change the response for API calls mentioned in the bug >> filed by David. We will just leave Apis the way it is. The original bug >> should be closed as WontFix, and I already reverted the commit. >> >> I didn't quite get your question about other APIs, could you please >> elaborate? >> >> -Alena. >> >> On 3/4/14, 10:28 AM, "Daan Hoogland" <daan.hoogl...@gmail.com> wrote: >> >>>We can not change the response for this api call? so other apis aren't >>>touched? >>> >>>On Tue, Mar 4, 2014 at 6:31 PM, Alena Prokharchyk >>><alena.prokharc...@citrix.com> wrote: >>>> Daan, you can¹t extend the response with more parameters as its a >>>>generic >>>> SuccessResponse used by many Apis, and you can¹t add anything a >>>>particular >>>> call specific to that. >>>> >>>> I think we shouldn¹t fix the bug at this point. You can always get the >>>> additional information you need by executing corresponding list* call. >>>> >>>> -Alena. >>>> >>>> On 3/4/14, 12:05 AM, "Daan Hoogland" <daan.hoogl...@gmail.com> wrote: >>>> >>>>>H Alena, >>>>> >>>>>You are right. I was under the impression that response format was >>>>>being extended. >>>>> >>>>>I saw you already reverted. If the response was extended with the >>>>>extra value it would be alright, would it? >>>>> >>>>>On Mon, Mar 3, 2014 at 7:06 PM, Alena Prokharchyk >>>>><alena.prokharc...@citrix.com> wrote: >>>>>> Daan, this fix break API compatibility! All the customers using >>>>>>these >>>>>>API, >>>>>> will end up with broken code on their side. As the response format >>>>>>is >>>>>> changed. Can you please roll it back? >>>>>> >>>>>> Thanks, >>>>>> Alena. >>>>>> >>>>>> On 3/3/14, 2:03 AM, "daan Hoogland" <daan.hoogl...@gmail.com> wrote: >>>>>> >>>>>>> >>>>>>>----------------------------------------------------------- >>>>>>>This is an automatically generated e-mail. To reply, visit: >>>>>>>https://reviews.apache.org/r/17591/#review35953 >>>>>>>----------------------------------------------------------- >>>>>>> >>>>>>>Ship it! >>>>>>> >>>>>>> >>>>>>>ebcaec8632dbd92c071317f3190915244a287afb >>>>>>> >>>>>>>- daan Hoogland >>>>>>> >>>>>>> >>>>>>>On Jan. 31, 2014, 2:51 p.m., David Grizzanti wrote: >>>>>>>> >>>>>>>> ----------------------------------------------------------- >>>>>>>> This is an automatically generated e-mail. To reply, visit: >>>>>>>> https://reviews.apache.org/r/17591/ >>>>>>>> ----------------------------------------------------------- >>>>>>>> >>>>>>>> (Updated Jan. 31, 2014, 2:51 p.m.) >>>>>>>> >>>>>>>> >>>>>>>> Review request for cloudstack. >>>>>>>> >>>>>>>> >>>>>>>> Bugs: CLOUDSTACK-5872 >>>>>>>> https://issues.apache.org/jira/browse/CLOUDSTACK-5872 >>>>>>>> >>>>>>>> >>>>>>>> Repository: cloudstack-git >>>>>>>> >>>>>>>> >>>>>>>> Description >>>>>>>> ------- >>>>>>>> >>>>>>>> CLOUDSTACK-5872: Async response from addAccountToProject doesn't >>>>>>>>contain useful information >>>>>>>> >>>>>>>> Updated the following classes to return a project object after >>>>>>>>async >>>>>>>>jobs complete: >>>>>>>> api/src/com/cloud/projects/ProjectService.java | 6 ++-- >>>>>>>> .../user/account/AddAccountToProjectCmd.java | 7 +++-- >>>>>>>> .../user/account/DeleteAccountFromProjectCmd.java | 7 +++-- >>>>>>>> .../user/project/UpdateProjectInvitationCmd.java | 8 +++-- >>>>>>>> server/src/com/cloud/projects/ProjectManager.java | 2 +- >>>>>>>> .../src/com/cloud/projects/ProjectManagerImpl.java | 34 >>>>>>>>+++++++++++----------- >>>>>>>> .../com/cloud/projects/MockProjectManagerImpl.java | 16 >>>>>>>>+++++----- >>>>>>>> >>>>>>>> Previously these API commands only returned "success => true" in >>>>>>>>the >>>>>>>>aysnc job result. Now it returns the project that a user was >>>>>>>>added/deleted to. >>>>>>>> >>>>>>>> >>>>>>>> Diffs >>>>>>>> ----- >>>>>>>> >>>>>>>> api/src/com/cloud/projects/ProjectService.java dc882ef >>>>>>>> >>>>>>>>api/src/org/apache/cloudstack/api/command/user/account/AddAccountTo >>>>>>>>Pr >>>>>>>>oj >>>>>>>>ec >>>>>>>>tCmd.java 36df579 >>>>>>>> >>>>>>>>api/src/org/apache/cloudstack/api/command/user/account/DeleteAccoun >>>>>>>>tF >>>>>>>>ro >>>>>>>>mP >>>>>>>>rojectCmd.java f6aa36c >>>>>>>> >>>>>>>>api/src/org/apache/cloudstack/api/command/user/project/UpdateProjec >>>>>>>>tI >>>>>>>>nv >>>>>>>>it >>>>>>>>ationCmd.java dda7b54 >>>>>>>> server/src/com/cloud/projects/ProjectManager.java f568146 >>>>>>>> server/src/com/cloud/projects/ProjectManagerImpl.java 5a0ed1c >>>>>>>> server/test/com/cloud/projects/MockProjectManagerImpl.java >>>>>>>>dc377ff >>>>>>>> >>>>>>>> Diff: https://reviews.apache.org/r/17591/diff/ >>>>>>>> >>>>>>>> >>>>>>>> Testing >>>>>>>> ------- >>>>>>>> >>>>>>>> Testing done on master. >>>>>>>> >>>>>>>> >>>>>>>> Thanks, >>>>>>>> >>>>>>>> David Grizzanti >>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>>> >>>>> >>>>>-- >>>>>Daan >>>> >>> >>> >>> >>>-- >>>Daan >> > > > >-- >Daan
