I don't think line 104 of libaiscf.py is what you want.
new_service() in libaiscf_instance.c returns "None".
That makes sense. But that means 104 should be two lines:
super(AISCF,self).new_service(self, service_name) # go create it
return AIservice(self, service_name) # look it up, may throw
KeyError
A doc string explaining that a KeyError is possible would help too.
Its understandable that it could throw this since you don't cache SMF
values (which is good), but that might surprise folks a little.
Otherwise its looking pretty good. Well apart from camelCase ;)
But then you got the constant LHS test from my examples which
Jean doesn't like (I use it because the compiler won't allow a
constant expression on LHS of *assignment* which is a class
of bug I seem to create).
-dvd
On Aug 21, 2009, at 12:07 AM, Clay Baenziger wrote:
> Hi Jean,
> Thank you for looking this thing over! My comments are in-line.
>
> New webrev:
>
> Full:
> http://cr.opensolaris.org/~clayb/10740/webrev3
>
> Diff from webrev 2:
> http://cr.opensolaris.org/~clayb/10740/webrev_2_3_diff/
>
> Thank you,
> Clay
>
> On Thu, 20 Aug 2009, jeanm wrote:
>
>> Nit: libaiscf.py line 64: service is misspelled.
>
> Fixed
>
>> libaiscf_pymod/Makefile: copyright date should be 2009
>
> Grr, I thought I'd gotten that but I've ensured it's changed now...
>
>> General: Please add header block comments for all new methods. Some
>> are there, some aren't.
>
> I added header blocks to every function now I believe
>
>> libaiscf_instance.c line 411-412: Why the empty if?
>
> Good question, it's now set to if (ret != AI_SUCCESS) with no else.
>
>> libaiscf_service.c line201 and 209: Any reason why you did if (0 !
>> = function()) and not if (function != 0) ?
>> Most people will be more used to seeing the later which makes it
>> easier to read.
>
> Yes, it's a rather long block that we're checking though. We're
> checking the return, so it's easier for one to see what we're
> checking it against than burrying the != 0 down below.
>
>> Jean
>>
>> Clay Baenziger wrote:
>>> Hi all,
>>> Per some initial feedback from Jean and Dave I've come to round
>>> two. Here are a few fixes for Python objects which had reference
>>> counts which were wrong (memory leaks) and I've added support for
>>> specifying the FMRI for AISCF() objects bringing them closer to
>>> being fully general.
>>> The diffs can be found at:
>>> http://cr.opensolaris.org/~clayb/10740/webrev_1_2_diff/
>>> Or the full webrev at:
>>> http://cr.opensolaris.org/~clayb/10740/webrev2/
>>>
>>> Thank you,
>>> Clay
>>> On Wed, 19 Aug 2009, Clay Baenziger wrote:
>>>> Hi all,
>>>> I believe the code is stable, clean, builds correctly and
>>>> ready for a full code review. I've implemented the last necessary
>>>> for the library. Now, one can view an instance's state (online,
>>>> offline, maintenance, etc.) and one can change the state too. For
>>>> example:
>>>>>>> libaiscf.AISCF().state
>>>> 'online'
>>>>>>> libaiscf.AISCF().state="DISABLE"
>>>> [wait a bit (~5 sec), or it'll return online still]
>>>>>>> libaiscf.AISCF().state
>>>> 'offline'
>>>> Webrev now at:
>>>> http://cr.opensolaris.org/~clayb/10740/webrev/
>>>>
>>>> Thank you,
>>>> Clay
>>>> On Tue, 18 Aug 2009, Clay Baenziger wrote:
>>>>> Hi all,
>>>>> I've written a Python<->C bridge for libaiscf. It provides a
>>>>> few ways to interact with SMF. I'm looking for some folks to
>>>>> look the code over for memory management issues, for any
>>>>> potential SMF issues or any other issues others might see. This
>>>>> code has not be whacked to be c-style happy, so don't worry
>>>>> about that unless you really want.
>>>>> I am having issues with the delete section of
>>>>> AIservice_set_subscript(), if anyone has any suggestions what
>>>>> might be happening (dbx stack trace included). This is triggered
>>>>> when one tries to delete a property via:
>>>>> del(libaiscf.AIservice(libaiscf.AISCF(),"test")['status'])
>>>>>
>>>>> Thank you,
>>>>> Clay
>>>>> Webrev is at:
>>>>> http://cr.opensolaris.org/~clayb/10740/webrev_prelim1/
>>>>> How the module works:
>>>>> ---------------------
>>>>> To load the module, one runs
>>>>> import libaiscf
>>>>> Then to create an SMF instance object, one can run
>>>>> (for svc:/system/install/server:default)
>>>>> instance=libaiscf.AISCF()
>>>>> (for svc:/system/install/server:someThingElse)
>>>>> instance=libaiscf.AISCF("someThingElse")
>>>> This has changed to:
>>>> instance=libaiscf.AISCF(instance="someThingElse")
>>>>> Further, to create an AI service object, one runs:
>>>> For clarification, this is just an SMF property group
>>>> representation
>>>>> service=libaiscf.AIservice(instance,"serviceName")
>>>>> To see a property key under the service, one can do:
>>>>> service['boot-dir'] (which returns the string value or a
>>>>> KeyError as a
>>>>> dictionary would if the key doesn't exist)
>>>>> To set or change a property under the service, one can do:
>>>>> serivce['boot-dir']="/var/ai/service1_image"
>>>>> All actions query SMF and no data is cached in case something
>>>>> changes under the consumer. All actions are handed to SMF when
>>>>> executed. Other functions implemented can be read in the PyDoc
>>>>> output for the module, here attached.
>>
>>
