Sounds good to me.. thanks to both of you for the input. When you get time if ya'll could review it formally on gerrit that would be appreciated.
Thanks, Brad On Tue, Oct 11, 2011 at 11:51 AM, Edgar Magana (eperdomo) <[email protected]> wrote: > 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
