On Fri, 2012-10-26 at 16:13 -0400, Rob Crittenden wrote:
> Simo Sorce wrote:
> > From: Simo Sorce <sso...@redhat.com>
> >
> > This is done as a default action of the ancestor class so that no matter 
> > what
> > platform is currently used this code is always the same and the name is the
> > wellknown service name.
> > This information will be used by ipacl to stop only and all the services 
> > that
> > have been started by any ipa tool/install script
> I think in the start method I'd rather test to see if the file exists 
> before trying to open it and do something (logging, return error, 
> something) if an exception is hit.

Why ?
If there is any fatal exception it will be caught in the following open().

> This will help us out of there are permission, disk space or other 
> problems writing this file.

Yeah but we get that in the next exception anyway.

> Similarly I wonder if there should be a try/except around updating the file.

I've let them explicitly out of try blocks because I want the method to
fail and report to the caller, would you rather use explict try:/except:
and a then a raise() again ?

> Maybe not a big deal but this patch isn't testable by itself because 
> patch 3 contains the spec addition of /var/run/ipa.

True, should I move the spec file change to this patch ?


Simo Sorce * Red Hat, Inc * New York

Freeipa-devel mailing list

Reply via email to