From: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; Add support for passing params for GET method, This include add param to get_list method, and filter the result with given params. we will call it like: GET /collection?filter_field=value With current implementation with a wrapper, we can easily support any collection query with param. Signed-off-by: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; --- src/kimchi/control/base.py | 28 ++++++++++++++++++++-------- src/kimchi/control/storagepools.py | 4 ++-- src/kimchi/control/storagevolumes.py | 2 +- src/kimchi/control/utils.py | 2 ++ 4 files changed, 25 insertions(+), 11 deletions(-) diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py index 185c8d8..a477abb 100644 --- a/src/kimchi/control/base.py +++ b/src/kimchi/control/base.py @@ -28,7 +28,7 @@ import urllib2 import kimchi.template from kimchi.control.utils import get_class_name, internal_redirect, model_fn -from kimchi.control.utils import parse_request, validate_method +from kimchi.control.utils import parse_request, parse_param, validate_method from kimchi.control.utils import validate_params from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import MissingParameter, NotFoundError, OperationFailed @@ -212,10 +212,10 @@ class Collection(object): return res.get() - def _get_resources(self): + def _get_resources(self, filter_params): try: get_list = getattr(self.model, model_fn(self, 'get_list')) - idents = get_list(*self.model_args) + idents = get_list(*self.model_args, **filter_params) res_list = [] for ident in idents: # internal text, get_list changes ident to unicode for sorted @@ -234,19 +234,31 @@ class Collection(object): args = self.resource_args + [ident.decode("utf-8")] return self.resource(self.model, *args) - def get(self): - resources = self._get_resources() + def filter_data(self, resources, filter_params): data = [] for res in resources: - data.append(res.data) + valid = True + for key, val in filter_params.items(): + if key in res.data and res.data[key] != val: + valid = False + break + if valid: + data.append(res.data) + return data + + def get(self, filter_params): + resources = self._get_resources(filter_params) + data = self.filter_data(resources, filter_params) return kimchi.template.render(get_class_name(self), data) @cherrypy.expose - def index(self, *args): + def index(self, *args, **kwargs): method = validate_method(('GET', 'POST')) if method == 'GET': try: - return self.get() + filter_params = parse_param() + validate_params(filter_params, self, 'get_list') + return self.get(filter_params) except InvalidOperation, param: error = "Invalid operation: '%s'" % param raise cherrypy.HTTPError(400, error) diff --git a/src/kimchi/control/storagepools.py b/src/kimchi/control/storagepools.py index 782f5a6..e3236a7 100644 --- a/src/kimchi/control/storagepools.py +++ b/src/kimchi/control/storagepools.py @@ -63,9 +63,9 @@ class StoragePools(Collection): return resp - def _get_resources(self): + def _get_resources(self, filter_params): try: - res_list = super(StoragePools, self)._get_resources() + res_list = super(StoragePools, self)._get_resources(filter_params) # Append reserved pools isos = getattr(self, ISO_POOL_NAME) isos.lookup() diff --git a/src/kimchi/control/storagevolumes.py b/src/kimchi/control/storagevolumes.py index d541807..cd15bcc 100644 --- a/src/kimchi/control/storagevolumes.py +++ b/src/kimchi/control/storagevolumes.py @@ -70,7 +70,7 @@ class IsoVolumes(Collection): super(IsoVolumes, self).__init__(model) self.pool = pool - def get(self): + def get(self, filter_params): res_list = [] try: get_list = getattr(self.model, model_fn(self, 'get_list')) diff --git a/src/kimchi/control/utils.py b/src/kimchi/control/utils.py index c3c5f8e..4b3c4b0 100644 --- a/src/kimchi/control/utils.py +++ b/src/kimchi/control/utils.py @@ -81,6 +81,8 @@ def parse_request(): raise cherrypy.HTTPError(415, "This API only supports" " 'application/json'") +def parse_param(): + return cherrypy.request.params def internal_redirect(url): raise cherrypy.InternalRedirect(url.encode("utf-8")) -- 1.8.1.2

On 01/14/2014 01:47 PM, lvroyce0210(a)gmail.com wrote: If the filter will be done in filter_data() below we don't need to pass filter_params to get_list You should continue the loop even when one resource doesn't have the filter parameter. For example, some storagepools have "task_id" parameter and other ones don't have. You don't need a function for it. It is just one line. Use cherrypy.request.params directly

From: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; Add a testcase to test GET param passing to demo how GET param work with current model implementation. Signed-off-by: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; --- tests/test_rest.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/test_rest.py b/tests/test_rest.py index 8732781..4d161d9 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -1132,6 +1132,40 @@ class RestTests(unittest.TestCase): self.assertIn('net_recv_rate', stats) self.assertIn('net_sent_rate', stats) + def test_get_param(self): + def hack_model(func): + def _get_param_func(*args, **kwargs): + res = func() + return res + return _get_param_func + + global model + old_handler = model.vms_get_list + model.vms_get_list = hack_model(model.vms_get_list) + + req = json.dumps({'name': 'test', 'cdrom': '/nonexistent.iso'}) + self.request('/templates', req, 'POST') + + # Create a VM + req = json.dumps({'name': 'test-vm1', 'template': '/templates/test'}) + resp = self.request('/vms', req, 'POST') + self.assertEquals(201, resp.status) + req = json.dumps({'name': 'test-vm2', 'template': '/templates/test'}) + resp = self.request('/vms', req, 'POST') + self.assertEquals(201, resp.status) + + resp = request(host, port, '/vms') + self.assertEquals(200, resp.status) + res = json.loads(resp.read()) + self.assertEquals(2, len(res)) + + resp = request(host, port, '/vms?name=test-vm1') + self.assertEquals(200, resp.status) + res = json.loads(resp.read()) + self.assertEquals(1, len(res)) + self.assertEquals('test-vm1', res[0]['name']) + + model.vms_get_list = old_handler class HttpsRestTests(RestTests): """ -- 1.8.1.2

On 2014年01月18日 02:07, Aline Manera wrote: This wrapper is for I pass filter param to get_list(). I will change it in next version and dump this wrapper.

Reviewed-by: Aline Manera <alinefm(a)linux.vnet.ibm.com&gt; On 01/14/2014 01:47 PM, lvroyce0210(a)gmail.com wrote:

On 2014年01月18日 02:45, Aline Manera wrote: I added this in previous version but deleted at last. Because for storage server, we only care about address, which can be get from /storageservers?query_param This resource yeilds no other information and should not expose to users.

Reviewed-by: Aline Manera <alinefm(a)linux.vnet.ibm.com&gt; On 01/14/2014 01:47 PM, lvroyce0210(a)gmail.com wrote:

On 01/14/2014 11:48 PM, lvroyce0210(a)gmail.com wrote: not all types of pools have ['source']['addr'], so it will cause a KeyError. Same here. And I am also wondering any pool doesn't have 'source'?

On 2014年01月18日 02:24, Aline Manera wrote: True, but can we add iscsi in a independant patch after nfs is merged? I would like this framework support merged first, based on a agreed framework, the other type can be easily accomplished and more focus on its functionality. No, it is not libvirt response, it is kimchi defined data, see commit acbcf04b8892b1c8876344 which I added to parse storage source information. ACK Same as above. Same as above. Scenario is : GET /storageservers?target_type=netfs return: { {'host': 'localhost', 'type': 'netfs, iscsi'} } Is that necessary? Users only care about servers.

Reviewed-by: Aline Manera <alinefm(a)linux.vnet.ibm.com&gt; On 01/14/2014 01:48 PM, lvroyce0210(a)gmail.com wrote:

On 2014年01月18日 02:46, Aline Manera wrote: GET /storageservers/*:name*/storagetargets?target_type=netfs returns 2 targets: [{'host': 'localhost', 'type': 'netfs', 'path':'/abcde'},{'host': 'localhost', 'type': 'netfs', 'path':'/efgh'}] Its already complete information. Do we need more information for individual target:{'host': 'localhost', 'type': 'netfs', 'path':'/abcde'}? I can't figure out what information that will be.

On 01/14/2014 01:48 PM, lvroyce0210(a)gmail.com wrote: Need to allow 'iscsi' too

On 01/14/2014 11:48 PM, lvroyce0210(a)gmail.com wrote: the above statement should belong to the if block. You could use lxm.builder to generate XML from arguments.

On 2014年01月18日 02:48, Aline Manera wrote: I agree, normally, we list the get list returned parameter under the API.md. But now, as added GET accept parameter, I added that in API.md, so the API.md parameter and response need to be refactored. I will do it in a single patch if you agree.