On Thu, Jul 16, 2009 at 07:09:31PM -0700, Brock Pytlik wrote:
> Edward Pilatowicz wrote:
>> i've updated the webrev with your feedback.
>> my replies are inline below.
>>
>> ed
>>
>>
>> On Thu, Jul 16, 2009 at 05:06:49PM -0700, Brock Pytlik wrote:
>>
>>> Edward Pilatowicz wrote:
>>>
>>>> hey all,
>>>>
>>>> it's taken me longer than i hoped, but i finally have a webrev ready
>>>> that implements change variant functionality. you can check it out
>>>> here:
>>>> http://cr.opensolaris.org/~edp/pkg-vc/
>>>>
>>>> this is my first shot at ips and python development, so i've probably
>>>> done a lot of bone headed things in this wad. please feel free to point
>>>> them out. i've also carried over a lot of ON and C 'isms with me, so
>>>> please feel free to try and beat them out of me (i promise not to
>>>> protest TOO much. ;).
>>>>
>>>> thanks
>>>> ed
>>>> _______________________________________________
>>>> pkg-discuss mailing list
>>>> [email protected]
>>>> http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
>>>>
>>>>
>>> General comment:
>>> I really like how you cleaned up and modularized the code in client.py
>>> and api.py. Thanks for doing that. ...
>>>
>>
>> cool. i didn't know how those changes would be recieved.
>>
>>
>>> ... In the same spirit, (client.py) lines
>>> 619-650 could probably be extracted and shared between different
>>> planning functions as well since they encounter a similar set of
>>> exceptions. We could pass in the function of the object we're going to
>>> call and the list of arguments. Whether or not we should do that is a
>>> different issue, and I know my ideas on code elegance aren't always
>>> sane, but it might centralize our code even further.
>>>
>>>
>>
>> well, unfortunatly the planning functions are all pretty different wrt
>> args and return values. what is pretty similar between them is the
>> exception handling. so what i could do is move all the exception
>> handlers into one function that could be invoked whenever a planning
>> function induced an exception (similar to what i did with
>> __plan_common_exception()).
>>
>> if this sound good to you let me know and i'll do it.
>>
>>
> Well, let me show you roughly what I had in mind:
> __plan_func(func, *args, **kwargs):
> try:
> ret = func(*args, **kwargs)
> return True (or success, or whatever that means no exception
> happened), ret
> except <and here add in all the exception handlers>
>
>
> then in install (for example):
>
> success, ret_val = __plan_func(api_inst.plan_install, pkg_list, filters,
> refresh_catalogs, noexecute, verbose=verbose,
> update_index=update_index)
> if not success:
> return 1 (or success, or whatever)
>
> stuff_to_do, cre = ret_val
> ... continue on with code.
>
>
> Image update planning bit would become:
>
> success, ret_val = __plan_func(api_inst.plan_update_all, sys.argv[0],
> refresh_catalogs,
> noexecute, force=force, verbose=verbose,
> update_index=update_index, be_name=be_name)
> if not success:
> return 1 (or success, or whatever)
>
> stuff_to_do, opensolaris_image, cre = ret_val
> ... continue on with code.
>
> (Sorry if that got mangled by email, if it did, let me know and I'll put
> a webrev together or paste it somewhere.)
>
> Now, what I'm not sure is whether making that change is a good/cool use
> of python, or a cringe inducing nightmare ;) It should work though
> (modulo typos).
>
>
so i tried doing this with function pointer. it didn't really help much
with readabiliy. so in the end i simply created a common exception
handling function so now we'll have:
try
rv1, rv2 = api_inst.plan_foo(...)
except:
__api_plan_exception(...)
i'll be sending out a new webrev shortly.
ed
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
