Re: [Pypes-developer] Reserved Attributes and Metadata Keywords for Packets
Status: Beta
Brought to you by:
egaumer
Menu
▾
▴
Re: [Pypes-developer] Reserved Attributes and Metadata Keywords for Packets
|
From: Matt W. <ma...@ma...> - 2009-09-20 21:53:55
|
See feature request 2860459 for a patch that includes all the
refactored components.
Thanks,
Matt Weber
On Sep 20, 2009, at 12:08 PM, Matt Weber wrote:
> Attached is a patch that completely refactors the packet api to be
> PEP-8 compliant. I have also added unit tests for every single
> method to ensure that it works 100% as expected. During the
> refactor, I removed some of the methods I felt were unnecessary due
> to duplicated functionality. For example I removed GetValue, and
> moved its functionality into Get. Why would we need a Get that
> raises a KeyError, or a GetValue that returns a different value? We
> can always use GetValue and check for the default. In FAST stages,
> I never used anything but GetValue. If you disagree, it will be
> trivial to add these methods back. Please review the patch and let
> me know what you think before I start refactoring the components to
> work with this api.
>
> Thanks,
>
> Matt Weber
>
> <pypes-2860459.patch>
>
> On Sep 19, 2009, at 9:03 AM, Matt Weber wrote:
>
>> I wanted to use the current GetMeta just like said with the default
>> attribute name, etc, but didn't want to break any components, so I
>> just adapted it to work. I definitely think this is how metadata
>> should work though.
>>
>> As far as the method names go.... I absolutely HATE the camel
>> case. As you said, your intentions were good, but I don't think it
>> is necessary. FAST is old and clunky, and people are going to have
>> to do some type of refactoring, so changing a few method names is
>> not that big of a deal. All of the core pypes code should follow
>> PEP-8 standards. I will refactor this patch and all the
>> components that break from the change.
>>
>> Also, do we really need all these comments for each method:
>>
>> @status: stable
>> @since: Jul 18 2009
>> @author: U{Eric Gaumer<mailto:eg...@ma...>}
>>
>> I want things like description and input/output parameters, but all
>> these tags are redundant and basically same the same thing. I
>> think these particular things can be dropped because it is going to
>> be hard to keep them up to date, and they really add no value. Can
>> I remove them?
>>
>> Thanks,
>>
>> Matt Weber
>>
>> On Sep 19, 2009, at 5:29 AM, Eric Gaumer wrote:
>>
>>>
>>> On Sep 19, 2009, at 1:58 AM, Matt Weber wrote:
>>>
>>>> I wanted to added packet level metadata, so I created a patch that
>>>> sets packet metadata to a special attribute called '__meta__'.
>>>> Now,
>>>> this can be a problem if someone wants to use '__meta__' as an
>>>> attribute in their packet. This is also a current problem with
>>>> metadata because internally attribute values for packets are
>>>> stored in
>>>> a dictionary field called 'value'. Metadata for attributes are
>>>> stored
>>>> in whatever meta key you specify. So, if someone to set a metadata
>>>> field called 'value' on any field, it will break the packet api.
>>>>
>>>> To resolve these problems, '__meta__' should be a reserved
>>>> attribute
>>>> and 'value' should be a reserved metadata keyword. When you
>>>> attempt
>>>> to set a reserved attribute or meta key, you should get KeyError.
>>>>
>>>> I implemented this functionality in the patch attached to bug
>>>> 2860459. The patch completely refactors the internal packet api to
>>>> support this functionality. All unit tests pass, as do the new
>>>> ones I
>>>> added. Also note that I refactored the 'value' key into
>>>> '__value__'
>>>> to be consistent with my '__meta__' attribute and reduce the
>>>> chances
>>>> for a name conflict.
>>>>
>>>> What do you think of this "reserved" concept? See http://sourceforge.net/tracker/?func=detail&aid=2860459&group_id=271766&atid=1155513
>>>
>>> Conceptually I think it's fine although I'm not sure the interface
>>> is very intuitive. As it exists today you have:
>>>
>>> GetMetaValue('myattr', 'metaname', default=None)
>>>
>>> Which basically says, get the meta value on the field myattr for
>>> metaname.
>>>
>>> You're proposing that the new interface would be:
>>>
>>> GetMetaValue(None, 'metaname', default=None)
>>>
>>> This seems unintuitive to me. Why not just have it be:
>>>
>>> GetMetaValue('metaname', default=None)
>>>
>>> Why have to pass in an attribute of None? It will always be None
>>> in the case of packet level meta-data so why not just create
>>> additional methods to handle packet level meta-data?
>>>
>>> Of course the above syntax wouldn't work since you can't overload
>>> methods in Python (at least not in the traditional C++ sense). So
>>> we have a few alternatives.
>>>
>>> GetPacketMetaValue('metaname', default=None)
>>>
>>> These long names are a bit frustrating in my opinion. What about:
>>>
>>> GetMetaValue('metaname', attr=None, default=None)
>>>
>>> If we flip the parameters then we can set a default value of None
>>> on the attribute.
>>>
>>> This whole Packet interface was designed to mirror the FAST
>>> docproc API. The idea being that this would make converting FAST
>>> pipeline stages simpler since the method signatures match (that's
>>> why we're using the camel case etc. here).
>>>
>>> Is it worthwhile to even do this? Does it really simplify things
>>> that much? I'd much rather see:
>>>
>>> get_meta_value('metaname', attr=None, default=None)
>>>
>>> So now we would have:
>>>
>>> get_meta_value('lang', 'title') # get me the language of the
>>> title attribute
>>> get_meta_value('lang') # get me the language of the packet
>>>
>>> This should be a pretty simple re-factor but it means we'll have
>>> to re-factor the existing components. If we're to do this then now
>>> is the time (before too many components get written or too many
>>> people start using this API).
>>>
>>> The question is do we really care to provide the same interface as
>>> FAST? I would argue it's not necessary. Their interface is
>>> antiquated and doesn't conform to PEP 8 standards.
>>>
>>> -Eric
>>>
>>>
>>>
>>>
>>> ------------------------------------------------------------------------------
>>> Come build with us! The BlackBerry® Developer Conference in
>>> SF, CA
>>> is the only developer event you need to attend this year.
>>> Jumpstart your
>>> developing skills, take BlackBerry mobile applications to market
>>> and stay
>>> ahead of the curve. Join us from November 9-12, 2009. Register
>>> now!
>>> http://p.sf.net/sfu/devconf_______________________________________________
>>> Pypes-developer mailing list
>>> Pyp...@li...
>>> https://lists.sourceforge.net/lists/listinfo/pypes-developer
>>
>> ------------------------------------------------------------------------------
>> Come build with us! The BlackBerry® Developer Conference in SF,
>> CA
>> is the only developer event you need to attend this year. Jumpstart
>> your
>> developing skills, take BlackBerry mobile applications to market
>> and stay
>> ahead of the curve. Join us from November 9-12, 2009. Register
>> now!
>> http://p.sf.net/sfu/devconf_______________________________________________
>> Pypes-developer mailing list
>> Pyp...@li...
>> https://lists.sourceforge.net/lists/listinfo/pypes-developer
>
> ------------------------------------------------------------------------------
> Come build with us! The BlackBerry® Developer Conference in SF, CA
> is the only developer event you need to attend this year. Jumpstart
> your
> developing skills, take BlackBerry mobile applications to market and
> stay
> ahead of the curve. Join us from November 9-12, 2009. Register
> now!
> http://p.sf.net/sfu/devconf_______________________________________________
> Pypes-developer mailing list
> Pyp...@li...
> https://lists.sourceforge.net/lists/listinfo/pypes-developer
Thanks,
Matt Weber
|