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

Reply via email to