Re: [Kimchi-devel] [PATCH 1/7] Granular Task Queries: Backend

On 17/04/2015 19:35, Christy Perez wrote: > > On 04/17/2015 01:39 PM, Aline Manera wrote: >> >> On 17/04/2015 15:08, Christy Perez wrote: >>> On 04/17/2015 12:51 PM, Aline Manera wrote: >>>> On 17/04/2015 12:24, Christy Perez wrote: >>>>> You can filter tasks by their state (running) and/or a target_uri. >>>>> If you want to see all cloning VM tasks, you can only query for >>>>> running tasks with /vm in the target_uri. This patch adds an action >>>>> keyword to the target_uris for existing tasks. This will make it >>>>> possible to query other types of tasks for the same resource. >>>>> >>>>> Signed-off-by: Christy Perez <christy(a)linux.vnet.ibm.com&gt; >>>>> --- >>>>> docs/API.md | 16 ++++++++++++++++ >>>>> src/kimchi/mockmodel.py | 7 ++++--- >>>>> src/kimchi/model/debugreports.py | 4 ++-- >>>>> src/kimchi/model/host.py | 4 ++-- >>>>> src/kimchi/model/storagepools.py | 2 +- >>>>> src/kimchi/model/storagevolumes.py | 12 +++++++----- >>>>> src/kimchi/model/vms.py | 5 ++--- >>>>> src/kimchi/model/vmsnapshots.py | 5 +++-- >>>>> 8 files changed, 37 insertions(+), 18 deletions(-) >>>>> >>>>> diff --git a/docs/API.md b/docs/API.md >>>>> index 3f7925f..1f8e769 100644 >>>>> --- a/docs/API.md >>>>> +++ b/docs/API.md >>>>> @@ -651,6 +651,22 @@ server. >>>>> * failed: The task failed >>>>> * message: Human-readable details about the Task status >>>>> * target_uri: Resource URI related to the Task >>>>> + The target uri may end with an action descriptor: >>>>> + * VM task descriptions: >>>>> + * clone >>>>> + * create >>>>> + * VM Snapshot description >>>>> + * create >>>>> + * Storage descriptions: >>>>> + * create >>>>> + * clone >>>>> + * Pool descriptions: >>>>> + * create >>>>> + * scan >>>> Is there a scan action? Something like /storagepools/<name>/scan ? >>> In StoragePoolsModel, def _do_deep_scan(self, params): >>> task_id = add_task('/storagepools/%s/scan' % ISO_POOL_NAME, >>> self.scanner.start_scan, self.objstore, >>> scan_params) >>>>> + * Debugreport descriptions: >>>>> + * create >>>>> + * Host SW Update descriptions: >>>>> + * update >>>> I don't think we need to change the main POST action. Just those >>>> actions >>>> to a specific resource. >>>> Example, >>>> >>>> # to create the VM >>>> POST /vms >>>> Target URI: /vms/<name> >>>> >>>> # to clone the VM >>>> POST /vms/<name>/clone >>>> Target URI: /vms/<name>/clone >>>> >>>> # to create a snapshot >>>> POST /vms/<name>/snapshot >>>> Target URI: /vms/<name>/snapshot/<snap> >>>> >>>> Does that make sense? >>> That makes sense. I can see how it can work without changing this, >>> but I >>> like adding it for consistency. If you then query all tasks, some will >>> not have the extra /<action> at the end. That will be understood to be >>> the POST (create), but, I like the consistency of having the action for >>> everything better. >>> >>> Also, the way it is now, you'll be able to query all 'create' tasks for >>> all objects across the system, which I think would be nice -- >>> although I >>> don't have a direct use case for that in mind. >> My suggestion is because we want to auto generate the target_uri based >> on request (in future) >> And in that case, we will not have the /create in the end. >> >> See: https://github.com/kimchi-project/kimchi/issues/559 > I see! So if we did keep it as it is, we'd have to add a little extra > logic in for that feature, like: > > def generate_target_uri(request): > target_uri = some_cool_one_liner > if request_type is 'POST': > target_uri = "%s/create" , %target_uri > return target_uri > > or something, which isn't as elegant. > > Another thing I just thought of is DELETE tasks. Right now those are all > done synchronously, but things should probably also be deleted > asynchronously in the future. Would that target uri also just be > /vms/<vm-name>? When generating the target URI automatically we can use the METHOD value to distinguish the request to a resource. But for now, I'd suggest to only change the target URI to add the action in the end. Just to make things simpler.

> >>>> >>>> >>>> >>>> >>>>> * **POST**: *See Task Actions* >>>>> >>>>> **Actions (POST):** >>>>> diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py >>>>> index 413ac5d..2b1b214 100644 >>>>> --- a/src/kimchi/mockmodel.py >>>>> +++ b/src/kimchi/mockmodel.py >>>>> @@ -244,7 +244,7 @@ def _get_volume_path(self, pool, vol): >>>>> return MockModel._libvirt_get_vol_path(pool, vol) >>>>> >>>>> def _gen_debugreport_file(self, name): >>>>> - return add_task('/debugreports/%s' % name, self._create_log, >>>>> + return add_task('/debugreports/%s/create' % name, >>>>> self._create_log, >>>>> self.objstore, name) >>>>> >>>>> def _create_log(self, cb, name): >>>>> @@ -333,7 +333,8 @@ def _mock_packageupdate_lookup(self, pkg_name): >>>>> return self._mock_swupdate.pkgs[pkg_name] >>>>> >>>>> def _mock_host_swupdate(self, args=None): >>>>> - task_id = add_task('/host/swupdate', >>>>> self._mock_swupdate.doUpdate, >>>>> + task_id = add_task('/host/swupdate/update', >>>>> + self._mock_swupdate.doUpdate, >>>>> self.objstore) >>>>> return self.task_lookup(task_id) >>>>> >>>>> @@ -385,7 +386,7 @@ def _mock_vm_clone(self, name): >>>>> def _mock_vmsnapshots_create(self, vm_name, params): >>>>> name = params.get('name', unicode(int(time.time()))) >>>>> params = {'vm_name': vm_name, 'name': name} >>>>> - taskid = add_task(u'/vms/%s/snapshots/%s' % (vm_name, name), >>>>> + taskid = add_task(u'/vms/%s/snapshots/%s/create' % (vm_name, >>>>> name), >>>>> self._vmsnapshots_create_task, >>>>> self.objstore, params) >>>>> return self.task_lookup(taskid) >>>>> >>>>> diff --git a/src/kimchi/model/debugreports.py >>>>> b/src/kimchi/model/debugreports.py >>>>> index 5f74da8..19b4de5 100644 >>>>> --- a/src/kimchi/model/debugreports.py >>>>> +++ b/src/kimchi/model/debugreports.py >>>>> @@ -62,8 +62,8 @@ def _gen_debugreport_file(self, name): >>>>> gen_cmd = self.get_system_report_tool() >>>>> >>>>> if gen_cmd is not None: >>>>> - return add_task('/debugreports/%s' % name, gen_cmd, >>>>> self.objstore, >>>>> - name) >>>>> + return add_task('/debugreports/%s/create' % name, >>>>> gen_cmd, >>>>> + self.objstore, name) >>>>> >>>>> raise OperationFailed("KCHDR0002E") >>>>> >>>>> diff --git a/src/kimchi/model/host.py b/src/kimchi/model/host.py >>>>> index 4419bb3..f429415 100644 >>>>> --- a/src/kimchi/model/host.py >>>>> +++ b/src/kimchi/model/host.py >>>>> @@ -115,8 +115,8 @@ def swupdate(self, *name): >>>>> raise OperationFailed('KCHPKGUPD0001E') >>>>> >>>>> kimchi_log.debug('Host is going to be updated.') >>>>> - taskid = add_task('/host/swupdate', swupdate.doUpdate, >>>>> self.objstore, >>>>> - None) >>>>> + taskid = add_task('/host/swupdate/update', >>>>> swupdate.doUpdate, >>>>> + self.objstore, None) >>>>> return self.task.lookup(taskid) >>>>> >>>>> def shutdown(self, args=None): >>>>> diff --git a/src/kimchi/model/storagepools.py >>>>> b/src/kimchi/model/storagepools.py >>>>> index b85f3b4..094c841 100644 >>>>> --- a/src/kimchi/model/storagepools.py >>>>> +++ b/src/kimchi/model/storagepools.py >>>>> @@ -167,7 +167,7 @@ def _do_deep_scan(self, params): >>>>> >>>>> params['path'] = >>>>> self.scanner.scan_dir_prepare(params['name']) >>>>> scan_params['pool_path'] = params['path'] >>>>> - task_id = add_task('/storagepools/%s' % ISO_POOL_NAME, >>>>> + task_id = add_task('/storagepools/%s/scan' % ISO_POOL_NAME, >>>>> self.scanner.start_scan, >>>>> self.objstore, >>>>> scan_params) >>>>> # Record scanning-task/storagepool mapping for future >>>>> querying >>>>> try: >>>>> diff --git a/src/kimchi/model/storagevolumes.py >>>>> b/src/kimchi/model/storagevolumes.py >>>>> index 0480496..9f93f10 100644 >>>>> --- a/src/kimchi/model/storagevolumes.py >>>>> +++ b/src/kimchi/model/storagevolumes.py >>>>> @@ -116,8 +116,10 @@ def create(self, pool_name, params): >>>>> raise InvalidParameter('KCHVOL0001E', {'name': name}) >>>>> >>>>> params['pool'] = pool_name >>>>> - targeturi = '/storagepools/%s/storagevolumes/%s' % >>>>> (pool_name, name) >>>>> - taskid = add_task(targeturi, create_func, self.objstore, >>>>> params) >>>>> + targeturi = '/storagepools/%s/storagevolumes/%s/create' \ >>>>> + % (pool_name, name) >>>>> + taskid = add_task(targeturi, create_func, self.objstore, >>>>> + params) >>>>> return self.task.lookup(taskid) >>>>> >>>>> def _create_volume_with_file(self, cb, params): >>>>> @@ -413,9 +415,9 @@ def clone(self, pool, name, new_pool=None, >>>>> new_name=None): >>>>> 'name': name, >>>>> 'new_pool': new_pool, >>>>> 'new_name': new_name} >>>>> - taskid = add_task(u'/storagepools/%s/storagevolumes/%s' % >>>>> - (pool, new_name), self._clone_task, >>>>> self.objstore, >>>>> - params) >>>>> + taskid = >>>>> add_task(u'/storagepools/%s/storagevolumes/%s/clone' % >>>>> + (pool, new_name), self._clone_task, >>>>> + self.objstore, params) >>>>> return self.task.lookup(taskid) >>>>> >>>>> def _clone_task(self, cb, params): >>>>> diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py >>>>> index 4c5f443..1ae841d 100644 >>>>> --- a/src/kimchi/model/vms.py >>>>> +++ b/src/kimchi/model/vms.py >>>>> @@ -332,9 +332,8 @@ def clone(self, name): >>>>> new_name = get_next_clone_name(current_vm_names, name) >>>>> >>>>> # create a task with the actual clone function >>>>> - taskid = add_task(u'/vms/%s' % new_name, self._clone_task, >>>>> - self.objstore, >>>>> - {'name': name, 'new_name': new_name}) >>>>> + taskid = add_task(u'/vms/%s/clone' % new_name, >>>>> self._clone_task, >>>>> + self.objstore, {'name': name, 'new_name': >>>>> new_name}) >>>>> >>>>> return self.task.lookup(taskid) >>>>> >>>>> diff --git a/src/kimchi/model/vmsnapshots.py >>>>> b/src/kimchi/model/vmsnapshots.py >>>>> index 3a92cdc..c4cc663 100644 >>>>> --- a/src/kimchi/model/vmsnapshots.py >>>>> +++ b/src/kimchi/model/vmsnapshots.py >>>>> @@ -72,8 +72,9 @@ def create(self, vm_name, params={}): >>>>> name = params.get('name', unicode(int(time.time()))) >>>>> >>>>> task_params = {'vm_name': vm_name, 'name': name} >>>>> - taskid = add_task(u'/vms/%s/snapshots/%s' % (vm_name, name), >>>>> - self._create_task, self.objstore, >>>>> task_params) >>>>> + taskid = add_task(u'/vms/%s/snapshots/%s/create' % (vm_name, >>>>> name), >>>>> + self._create_task, self.objstore, >>>>> + task_params) >>>>> return self.task.lookup(taskid) >>>>> >>>>> def _create_task(self, cb, params):