Clay,

svc-install-server
------------------
The installadm code shouldn't have to check for, and make this dir.
This is simply configuration we've set for our instance of Apache.
Apache creates its configured LockFile and PidFile dirs accordingly
on the fly.


one more comment below ...


Clay Baenziger wrote:
> Hi Ethan,
>     Good ideas. New webrev at 
> http://cr.opensolaris.org/~clayb/11214_12016/webrev1/ as I need to 
> potentially create the /var/run directory path so the SMF method is here
> updated.
>                             Thank you,
>                             Clay
> 
>> ai-httpd.conf
>> -------------
>> 36,37 - Shouldn't these paths be somewhere in /var/run instead of
>> somewhere on the disk?
> 
> I had thought more about keeping AI bits together, but /var/run is the 
> canonical place for such stuff, so it should be there. Now the next 
> question is, there's /var/run/apache2/2.2/httpd.pid, so we should use 
> that directory. Thank you for pointing out a tmp. directory is a much 
> better place for such transient data (not to mention where the ARC says 
> to put such stuffs).
> 
>> delete_service.py
>> -----------------
>> 809-812, Should we execute lines 814-819 first before we transition
>> to MAINTENANCE?
> 
> I don't see it one way or the other. The code would need to be slightly 
> different to do it the way you suggest -- not bad, just different.
> 
> As per Jack's request in the code walk-through I try not to instantiate 
> extra AISCF() objects and consistently access the list of services (here 
> service.instance (the AISCF() object its wedded to), then the services 
> property of that -- so service.instance.services). It simply saves 
> creating another AISCF() object.
> 
> There's no functional difference I'm aware of.

I guess its just a nit perhaps, but it seems more logical for us to
make sure we successfully deleted the service configuration before 
transitioning to maintenance.   All I'm suggesting is the below.
(I'm not seeing how this involves creating another AISCF() object)


     # remove the service
     try:
         service.instance.del_service(service.serviceName)
     # if the service can not be found a KeyError will be raised
         except KeyError:
             pass

     # if we just deleted the instance's last service, transition the
     # SMF instance to maintenance
     if len(service.instance.services) == 0:
         service.instance.state = "MAINTENANCE"



thanks,
-ethan



> 
>> thanks,
>> -ethan
>>
>>
>> Clay Baenziger wrote:
>>> Hi all,
>>>     Could Ethan (or if someone else wants to instead) code review the 
>>> changes for:
>>> 11214 - AI image web server won't run if apache22 is already enabled
>>> 12016 - Need to use SIGTERM and check for last service in delete_service
>>>
>>> The -- seven line -- webrev is at:
>>> http://cr.opensolaris.org/~clayb/11214_12016/
>>>
>>> The bugs are at:
>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=11214
>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=12016
>>>                             Thank you,
>>>                             Clay
>>

Reply via email to