Open Stack

Hi Mark, interesting points. Comments inline.

On 02/08/2013 10:15 AM, Mark McClain wrote:
> -1 on locals()
> 
> -1 on adding kwargs to OpenStack logging methods.  It's replacing one
> type of magic with another type.

I originally thought this as well. But then I thought of the following:

a) The code Jason demoed is all just using the existing logging module's
functionality, there's no magic really involved

b) We already have a custom logging handler that manages formatting of
tracebacks and audit messages

> It may not be perfect, but Python has a well established logging module.
>  When we make decisions to create custom variants of python internals
> with different method signatures we actually make this project harder
> for new developers to join.  We're also breaking one of the ideals of
> Python "There should be one-- and preferably only one --obvious way to
> do it."  This is from the Zen of Python (If you've never read it, start
> up the interpreter and type 'import this').  Having the option to pass a
> dict via the args param or using the proposed kwargs parameter violates
> this concept.

Yes, I thought about this as well, and weighed the importance of having
a single way of doing things. And I came to the conclusion that it was
better to have one way of calling LOG.debug() -- even with a custom
handler -- than to have the existing situation of using locals() some
places and using dicts other places.

> I also do not accept that dictionaries created in code have any
> readability impact when properly styled.  A method call with many
> keyword args is just as ugly.

True enough, it's a stylistic thing, and likely getting into bikeshed
territory ;)

> There are also performance considerations about constructing
> dictionaries and I'd like to point everyone to Doug Hellman's written
> article  about dictionary construction performance.
> 
> http://www.doughellmann.com/articles/misc/dict-performance/index.html

I don't buy that this would have a substantial impact compared to the
cost of writing the message to disk.

Best,

-jay

> mark
> 
> On Feb 8, 2013, at 7:02 AM, Sean Dague <sdague at linux.vnet.ibm.com
> <mailto:sdague at linux.vnet.ibm.com>> wrote:
> 
>> On 02/08/2013 12:13 AM, Jay Pipes wrote:
>>> On 02/07/2013 09:58 PM, Sean Dague wrote:
>>>> On 02/07/2013 06:04 PM, Nachi Ueno wrote:
>>>>>> I suspect we should just slowly transition as people touch code.
>>>>>
>>>>> I agree. The reason why I asked this is for reviewing.
>>>>> so I'll comment not to use local() in review.
>>>>
>>>> If we have rough concensus start with updating HACKING in the relevant
>>>> projects. Then I suggest a grind through replacing the code. Once it's
>>>> out we can enforce it in hacking.py (comming soon to a project near you,
>>>> and not just in nova and tempest).
>>>
>>> Go for it. Jason's suggestion for allowing LOG.debug() et al messages to
>>> allow kwargs would make things much more readable than supplying huge
>>> manually-coded dicts.
>>
>> +1, that was a nice suggestion.
>>
>> -- 
>> Sean Dague
>> IBM Linux Technology Center
>> email: sdague at linux.vnet.ibm.com <mailto:sdague at linux.vnet.ibm.com>
>> alt-email: sldague at us.ibm.com <mailto:sldague at us.ibm.com>
>>
>>
>> _______________________________________________
>> OpenStack-dev mailing list
>> OpenStack-dev at lists.openstack.org
>> <mailto:OpenStack-dev at lists.openstack.org>
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> 
> 
> 
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>