On 03/ 1/11 01:28 PM, Martin Widjaja wrote:
On 3/1/2011 11:03 AM, Keith Mitchell wrote:
On 03/ 1/11 10:11 AM, Martin Widjaja wrote:
Sue - In reviewing this quickly, it looks good overall, but I have
several questions, mostly to satisfy my own curiosity:
I know you asked Sue, but I have some answers to chime in with here.
- Makefile: would it be worth keeping the generic rules for building
.c, etc.? Maybe this can be useful for other things later on?
- installadm.py: 354: is there a reason to remove the traceback
limit here? Is this going to be too much if everything is printed?
If we reach a point where we're printing a traceback, we're already
in a scenario where the user encountered a bug. More information is
better than less, in that case (and the limit=2 almost always cut out
the useful portion of the traceback, since it cuts out the 'bottom'
of the stack).
- installadm_common.py: 95-105: is this ksh execution to determine
multi-home going to persist or is there a better way to gather this
information using either python lib or C lib? I am personally not a
fan of command line output parsing.
I was thinking of swapping that out with ISIM, though it may be quick
to change as part of this push. The concern I have with doing it
*right* now is it would be easy to miss edge cases - by porting this
right across from the .c implementation, we're at least not
introducing any new bugs.
OK. Sure, that's a good strategy.
- libaiscf.py: 105: what's wrong with previous implementation?
The old version was simply broken - there was an extraneous "self"
argument. Any attempt to call that function resulted in a traceback.
OK. I guess you cannot really call a function with "self", and that's
fine, but wouldn't something like this works:
return (AIservice(super(AISCF, self).new_service(service_name)))
I guess the part I don't really like/understand with having the call
to super class (which seems to be defined in C code
libaiscf_instance.c?) is that it's unclear how the AISCF.new_service
gets the return value from that superclass call, and unless the
service_name is actually populated with a value, it's not too obvious
what the return value here is.
If we explicitly return the whole thing like above (assuming
superclass actually return something), it is easier to understand.
But then again... maybe I missed something here.
The short answer is that libaiscf._AISCF.new_service doesn't actually
return anything. It actually just creates the property group in SMF.
(See lines 394-426 of usr/src/lib/libaiscf_pymod/libaiscf_instance.c)
As for the return value:
return AIService(self, service_name)
You get an AIService object associated with the installadm service named
"service_name" - the constructor for AIService takes two arguments: (1)
an AISCF instance (a representation of the
svc:/system/install/server:default SMF service instance) - in this case,
"self" is a reference to just such an object - and (2) a service_name.
In this case, the AIService instance returned is a reference to the
newly created installadm service's SMF property group.
- Keith
Thanks,
Martin
Thanks,
Martin
PS. test_ai_smf_service.py: can't get more optimistic than wobbly
knees and failing eyesight..
On 2/28/2011 11:44 AM, Sue Sohn wrote:
Please do a code review of the changes for:
7020631 Convert create-service.c to python
http://monaco.sfbay.sun.com/detail.jsf?cr=7020631
Webrev:
http://cr.opensolaris.org/~sohn/7020631
Thanks,
Sue
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
