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. -- Simo Sorce * Red Hat, Inc * New York _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
