[Freeipa-devel] [PATCH] 89 ipa-adtrust-install: allow to reset te NetBIOS domain name

[Martin Kosek]

On 10/30/2012 12:16 PM, Sumit Bose wrote:
> Hi,
> 
> this patch allows ipa-adtrust-install to reset the NetBIOS domain name
> and fixes https://fedorahosted.org/freeipa/ticket/3192 .
> 
> bye,
> Sumit
> 


Hello Sumit,

I found few issues with your patch:

1) It requires admin to be kinited ("conn.do_sasl_gssapi_bind()") I do not
think this is necessary, ipa-adtrust-install already requires admin password to
be passed and it already connects to LDAP with these credentials:

api.Backend.ldap2.connect(ccache.name)

You could use ipa.Backend.ldap2 object to do entry retrieval
(ipa.Backend.ldap2.get_entry) without a need to init IPAdmin at all.

2) When doing try..except statement, rule of thumb says that it should be as
short as possible, so that it does not hide other potential errors and makes
clear what function raises the catched exception.

In your case:

try:
    entry = api.Backend.ldap2.get_entry(DN(('cn', api.env.domain),
                                        api.env.container_cifsdomains,
                                        self.api.env.basedn),
                                       ['ipantflatname'])
except errors.NotFound:
    reset_netbios_name = False
else:
    # process entry

Should be a pattern that you want.

3) I think this line is redundant:
+                    print "Say 'yes' if the NetBIOS shall be changed and " \
+                          "'no' if the old one shall be kept."

IMO, the question:

reset_netbios_name = ipautil.user_input( 'Reset NetBIOS domain name?',  default
= False, allow_empty = False)

and the information printed before is enough.

Martin