cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alena Prokharchyk <Alena.Prokharc...@citrix.com>
Subject Re: [ACS41] Blocker and critical issues stopping me from spinning out a new RC for 4.1
Date Thu, 25 Apr 2013 17:48:02 GMT
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" <min.chen@citrix.com> 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" <tsp@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" <tsp@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
>>
>
>



Mime
View raw message