JobId/JobStatus were introduced to the list* responses on purpose. Some of the CS customers wanted to see what async jobs that are currently being executed on the object, just by looking at the result of list* command.
So removing async job params from the list response is not a good idea as customers' software might rely on its presence. On 4/25/13 10:32 AM, "Min Chen" <[email protected]> wrote: >Totally agree. > >Chip and others, Prasanna and I exchanged ideas about CLOUDSTACK--2126, >and we both agree that we should only return async job Id and job status >in queryAsyncJob and listAsyncJob commands, not in any other APIs so that >we can have consistent response return in various scenarios. If we fix >this way, the behavior for listXXX api is changed. In 4.0, CloudStack has >used ApiServer.buildAsyncListResponse() routine to fill in async job Id >and status for all listXXX Apis. With the proposed fix, we will not show >async job information in listXXX response as well. Not sure if anybody see >any side effect on this slight change from 4.0? Anybody know the reason >why we had that special code in 4.0 to return async job for listXXX api? >Since the proposed change will affect all newly created db view in 4.1, we >suggest that this can be fixed in 4.2 since current behavior is not >breaking functionalities, just return more information. Any thoughts? > >Thanks >-min > > > > > > >On 4/25/13 10:05 AM, "Prasanna Santhanam" <[email protected]> wrote: > >>That's odd - listXxx commands don't do async at all and shouldn't be >>generating jobstatus,jobid etc. So I'm not sure why that was added in >>(prior?) 4.0. I'd like to take that out too if there's no real reason >>behind it. >> >>-- >>Prasanna., >> >>On Thu, Apr 25, 2013 at 09:57:02AM -0700, Min Chen wrote: >>> In 4.0, CS has special code to return job status for a VM returned from >>> listVMsCmd. During API performance refactoring, I have a created a DB >>>view >>> user_vm_view that joins async_job table just for that purpose and used >>> that view to uniformly generate UserVmResponse. So the same code will >>>be >>> applied to generate UserVmResponse whenever it is used. In this case, >>> deployVMCmd itself will also return a UserVmResponse, thus the same >>>code >>> applied, and so that is what you see. If we all agree that job status >>> should not appear in UserVmResponse, then I can change the view to >>>remove >>> job from async_job. But I would argue that we should not return >>>jobStatus >>> in ListVmsCmd as well, this will also be a change from 4.0 release. >>> >>> Thanks >>> -min >>> >>> On 4/25/13 9:48 AM, "Prasanna Santhanam" <[email protected]> wrote: >>> >>> >On Thu, Apr 25, 2013 at 09:30:08AM -0700, Min Chen wrote: >>> >> Prasanna, I updated CLOUDSTACK-2126 with my comment. That is the >>> >>intended >>> >> change done in list API performance improvement work, and I don't >>>see >>> >>any >>> >> issues by having the consistent UserVmResponse for both deployVMCmd >>>and >>> >> listVMsCmd. Every BaseResponse class has jobId and jobStatus as >>> >>serialized >>> >> fields, I don't see why marvin has issues in deserialization in this >>> >>case. >>> >> Did I miss anything? >>> >> >>> > >>> >I'm not sure why internal representation should be a reason to surface >>> >it upwards. But that's not the part I'm concerned with: If you look at >>> >the response carefully - queryAsyncJobResultResponse contains two >>> >jobstatus attributes. One for the query job and one as part of the >>> >virtualmachine (within the virtualmachine block). The concern is with >>> >the latter. >>> > >>> >That block pasted for brevity: >>> > >>> >virtualmachine : { >>> > "id": "649663f7-3c8d-4e0d-b693-4b1ea6085a84", >>> > "name": "649663f7-3c8d-4e0d-b693-4b1ea6085a84", >>> > "account": "QX7KKV", >>> > ... >>> > .. >>> > "zoneid": "6e301be1-8010-4b57-9638-c90761e40dc9", >>> > "jobstatus": 0 <?My Problem?> >>> >} >>> > >>> >These attributes qualify a VM, but I'm not sure why jobstatus is in >>> >there. That's an attribute of the job itself which is CloudStack's >>> >concern, but not the VM's concern. When marvin looks to deserialize >>> >back to a VM object, it looks at the inner block only. I can >>> >workaround these within marvin, so feel free to reduce the priority if >>> >you think the bug can be fixed later. Just that jobstatus represented >>> >as a VM attribute doesn't seem right to me. >>> > >>> >Thanks, >>> > >>> >-- >>> >Prasanna., >>> > >>> >------------------------ >>> >Powered by BigRock.com >>> > >> >> >>------------------------ >>Powered by BigRock.com >> > >
