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" <t...@apache.org> 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" <t...@apache.org> 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
>
