Hi Brad, I like the idea of adding **kwargs and still keeping existing parameters. Thus, we just need to change plugin method's signature, instead of changing the method body itself.
For extended data parameters, we can pass in a flexible data structure. Thus, plugin provider can use it to pass any data in. What we need to enforce is the prefix for the attribute's name, which needs to that vendor's ID. Best, Ying > -----Original Message----- > From: Brad Hall [mailto:b...@nicira.com] > Sent: Tuesday, October 11, 2011 10:58 AM > To: Salvatore Orlando > Cc: Ying Liu (yinliu2); Dan Wendlandt; netstack@lists.launchpad.net > Subject: Re: [Netstack] Data extensions in quantum > > Replies inline.. > > On Tue, Oct 11, 2011 at 7:54 AM, Salvatore Orlando > <salvatore.orla...@eu.citrix.com> 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- > >> bounces+salvatore.orlando=eu.citrix....@lists.launchpad.net > >> [mailto:netstack- > >> bounces+salvatore.orlando=eu.citrix....@lists.launchpad.net] On > Behalf Of > >> Brad Hall > >> Sent: 10 October 2011 23:06 > >> To: Ying Liu (yinliu2); Dan Wendlandt > >> Cc: netstack@lists.launchpad.net > >> Subject: Re: [Netstack] Data extensions in quantum > >> > >> Ping :) Reviews/comments appreciated. > >> > >> Thanks, > >> Brad > >> > >> On Tue, Sep 27, 2011 at 9:29 AM, Brad Hall <b...@nicira.com> 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) > <yinl...@cisco.com> > >> 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: netstack-bounces+yinliu2=cisco....@lists.launchpad.net > >> >>> [mailto:netstack-bounces+yinliu2=cisco....@lists.launchpad.net] > On > >> >>> Behalf Of Brad Hall > >> >>> Sent: Thursday, September 22, 2011 3:20 PM > >> >>> To: netstack@lists.launchpad.net > >> >>> 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 : > >> >>> netstack@lists.launchpad.net Unsubscribe : > >> >>> https://launchpad.net/~netstack More help : > >> >>> https://help.launchpad.net/ListHelp > >> >> > >> > > >> > >> -- > >> Mailing list: https://launchpad.net/~netstack > >> Post to : netstack@lists.launchpad.net > >> Unsubscribe : https://launchpad.net/~netstack > >> More help : https://help.launchpad.net/ListHelp > > -- Mailing list: https://launchpad.net/~netstack Post to : netstack@lists.launchpad.net Unsubscribe : https://launchpad.net/~netstack More help : https://help.launchpad.net/ListHelp
