On Mon, 2012-10-29 at 16:19 -0400, Rob Crittenden wrote:
> Simo Sorce wrote:
> > 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().
> That's true, just seems kinda pointless to ignore an error only to try
> it again. That's fine, not a deal breaker to me.
> >> 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 ?
> Ok, I think the method docs should state that any exceptions are left
> for the caller to handle.
I will add a note I guess, where should I do it given this class is
abstract ? in base.py ?
> >> 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 ?
> No, not a big deal. Once it's all committed it's all good.
> The shutdown message when there is no services file is a bit strange though:
> # ipactl stop
> Failed to read data from Directory Service: [Errno 2] No such file or
> directory: '/var/run/ipa/services.list'
> Shutting down
Ah thanks, this was a copy/paste error, fixed in the patches I am
Simo Sorce * Red Hat, Inc * New York
Freeipa-devel mailing list