Hi Brad, After taking a look to both review diffs, indeed adding **kwargs makes a cleaner diff. If this change will be good enough, I think we should stay with it but if you are planning to make more changes I do agree with you that the sooner the better :-) then we will all have more time to make the necessary changes in the vendors Plugins.
Thanks, Edgar -----Original Message----- From: [email protected] [mailto:[email protected]] On Behalf Of Brad Hall Sent: Tuesday, October 11, 2011 10:58 AM To: Salvatore Orlando Cc: [email protected] Subject: Re: [Netstack] Data extensions in quantum Replies inline.. On Tue, Oct 11, 2011 at 7:54 AM, Salvatore Orlando <[email protected]> wrote: > I don't have yet performed a complete review. > Anyway, I was wondering whether we can avoid breaking existing plugins by > using **kwargs, and not removing existing parameters > > Example: create_network(self, tenant_id, net_name, **kwargs) Yeah, I had thought about doing it that way but figured if we were going to break plugins now would be a better time to do it then possibly later. I'm OK either way -- using kwargs makes for a much cleaner diff. I posted a new review here: https://review.openstack.org/#change,835 > I also think Ying had another important point on how the API layer should > parse information for extended data. > Parse_request_params only deal with 'core' structure of resources, and this > is something else that needs to be addressed. Hopefully she'll chime in. As it is parse_request_params does only look at any core items and just builds a dict with the rest of them. I think it should be up to the plugin to parse anything extra since parse_request_params is unlikely to know what it is. (Or, maybe, there could be a way for a plugin to register any extra params it would like to take..) > Salvatore Thanks for taking a look! -Brad >> -----Original Message----- >> From: netstack- >> [email protected] >> [mailto:netstack- >> [email protected]] On Behalf Of >> Brad Hall >> Sent: 10 October 2011 23:06 >> To: Ying Liu (yinliu2); Dan Wendlandt >> Cc: [email protected] >> Subject: Re: [Netstack] Data extensions in quantum >> >> Ping :) Reviews/comments appreciated. >> >> Thanks, >> Brad >> >> On Tue, Sep 27, 2011 at 9:29 AM, Brad Hall <[email protected]> wrote: >> > I've created a branch to address this here: >> > https://review.openstack.org/#change,665 >> > >> > Currently I've only add param dicts to the create calls but maybe it >> > makes sense to do it for the update calls as well. I'm not terribly >> > happy about changing the plugin interface as this will break any >> > plugins that aren't in the tree .. if anyone has other ideas that >> > would be appreciated. Comments/suggestions/etc appreciated. >> > >> > Thanks, >> > Brad >> > >> > On Fri, Sep 23, 2011 at 12:04 AM, Ying Liu (yinliu2) <[email protected]> >> wrote: >> >> Hi Brad, >> >> >> >> I agree with you. >> >> >> >> For extension framework, its functionality only provides us mechanism >> >> to get extended attributes into request data, which is done as the >> >> example illustrated. However, to pass the data to the plugin for >> >> further processing, we need either modify existing action's >> >> parse_request_param or having flexible param structure, eg. (key, >> >> value) pair. We actually proposed this earlier as it makes extension much >> easier. >> >> >> >> Just my 2 cents. >> >> >> >> Best, >> >> Ying >> >> >> >> >> >> >> >> >> >>> -----Original Message----- >> >>> From: [email protected] >> >>> [mailto:[email protected]] On >> >>> Behalf Of Brad Hall >> >>> Sent: Thursday, September 22, 2011 3:20 PM >> >>> To: [email protected] >> >>> Subject: [Netstack] Data extensions in quantum >> >>> >> >>> Hello Netstackers, >> >>> >> >>> I've been working with the extensions framework lately and wanted to >> >>> know how data extensions are supposed to work. What I want to do is >> >>> something like this: >> >>> >> >>> A hypothetical device that I'm writing a plugin for has a property >> >>> on the port called "foo" and I want to be able to create a port >> >>> giving "foo" a value. So based on the examples in >> >>> tests/test_extensions.py I made an extension that contains the >> following: >> >>> >> >>> @classmethod >> >>> def get_request_extensions(self): >> >>> def _foo_handler(req, res): >> >>> data = json.loads(res.body) >> >>> data[foo] = req.GET.get('foo') >> >>> res.body = json.dumps(data) >> >>> return res >> >>> >> >>> req_ext = extensions.RequestExtension('POST', >> >>> '/tenants/:(tenant_id)/networks/:(network_id)/ports', >> >>> _foo_handler) >> >>> return [req_ext] >> >>> >> >>> Then I can use: >> >>> >> >>> POST /v1.0/tenants/<tid>/networks/<nid>/ports (body = {'foo': >> >>> 'bar'}) >> >>> >> >>> The problem is that there is no way (currently) to propagate 'foo' >> >>> to the create_port call. The port ops_param_list only has >> >>> port-state so parse_request_params won't allow my 'foo' .. and even >> >>> if it did the create_port call only picks out the params it knows >> >>> about (port-state) and sends those directly to plugin::create_port(). >> >>> >> >>> What it seems like we need: >> >>> >> >>> 1) For parse_request_params we should add any extra params to the >> >>> result dictionary. >> >>> 2) {create,update}_{port,network}() calls should just send a params >> >>> dict as an argument to the plugin create/update function instead of >> >>> sending the params like port-state as explicit arguments. >> >>> >> >>> Does this make sense? Please let me know if I'm thinking about this >> >>> all wrong or if there is a different way this is supposed to be used. >> >>> >> >>> Thanks, >> >>> Brad >> >>> >> >>> -- >> >>> Mailing list: https://launchpad.net/~netstack Post to : >> >>> [email protected] Unsubscribe : >> >>> https://launchpad.net/~netstack More help : >> >>> https://help.launchpad.net/ListHelp >> >> >> > >> >> -- >> Mailing list: https://launchpad.net/~netstack >> Post to : [email protected] >> Unsubscribe : https://launchpad.net/~netstack >> More help : https://help.launchpad.net/ListHelp > -- Mailing list: https://launchpad.net/~netstack Post to : [email protected] Unsubscribe : https://launchpad.net/~netstack More help : https://help.launchpad.net/ListHelp -- Mailing list: https://launchpad.net/~netstack Post to : [email protected] Unsubscribe : https://launchpad.net/~netstack More help : https://help.launchpad.net/ListHelp
