[Freeipa-devel] [PATCH] 0043 fix ipa-dns-install to not require DM password

Jan Zelený jzeleny at redhat.com
Fri Jan 7 09:44:25 UTC 2011


Simo Sorce <ssorce at redhat.com> wrote:
> On Fri, 2011-01-07 at 09:53 +0100, Jan Zelený wrote:
> > Simo Sorce <ssorce at redhat.com> wrote:
> > > On Thu, 2011-01-06 at 10:35 +0100, Jan Zelený wrote:
> > > > Simo Sorce <ssorce at redhat.com> wrote:
> > > > > This patch makes it possible to run ipa-dns-install and use the
> > > > > admin kerberos credentials.
> > > > > 
> > > > > Fixes #686.
> > > > > 
> > > > > Simo.
> > > > 
> > > > Nack, I have some comments:
> > > > 
> > > > Exception handling (chunk #4):
> > > > Those prints should go away. But the main thing: that particular part
> > > > of code doesn't seem to produce any exceptions, which should be
> > > > handled
> > > > 
> > > > Function ldap_disconnect isn't used anywhere. That makes me wonder -
> > > > is it redundant or should it be somewhere in the code. I guess this
> > > > is a policy issue - either we want the connection to stay as long as
> > > > possible or we want to use it only for a certain set of commands and
> > > > then disconnect it.
> > > 
> > > Attached new patch that fixes hunk #4.
> > > Actually I ended up using ldap_disconnect() here as we need to test the
> > > ldap connection anyway.
> > > I also had to do minor changes to Bindinstance() as the code was
> > > clearing self.fqdn after Service.__init__ set it.
> > > 
> > > Simo.
> > 
> > Nack,
> > IMO in case invalid credentials are given and the script runs in
> > unattended mode, the script should exit and not ask for the password.
> 
> Good catch!
> 
> New patch attached.
> 
> Simo.

ack

Jan




More information about the Freeipa-devel mailing list