On 03/06/2015 02:08 PM, Jan Cholasta wrote:
Hi Martin,

Dne 6.3.2015 v 13:05 Martin Babinsky napsal(a):
This series of patches for the master/4.1 branch attempts to implement
some of the Rob's and Petr Vobornik's ideas which originated from a
discussion on this list regarding my original patch fixing
https://fedorahosted.org/freeipa/ticket/4808.

I suppose that these patches are just a first iteration, we may further
discuss if this is the right thing to do.

Below is a quote from the original discussion just to get the context:

1) Why 5 patches for 2 changes (kinit_hostprincipal instead of exec
kinit, ipa-client-install --kinit-attempts)?
Will squish them to a smaller number (2-3)
2) IMO a for loop would be better than an infinite while loop:

     for attempt in range(attempts):
         try:
             # kinit
             ...
         except krbV.Krb5Error as e:
             # kinit failed
             ...
         else:
             break
     else:
         # max attempts reached
         ...

That's true. Infinite loops are tad scary anyway.
3) I think it would be nice to support ccache types other than FILE.
According to Petr Vobornik (see his reply), the user is limited mostly to FILE ccache type, so I don't know if it will make sense to support also other types.

4) I would prefer if you kept using the full ccache name returned from
kinit_hostprincipal when connecting to LDAP.

5) Given that the ccache path usually ends with "/ccache", I would
retain the old way of calling kinit_hostprincipal. You can do something
like this to support all of the above:

     def kinit_hostprincipal(keytab, ccache_file, principal, attempts=1):
         if os.path.isdir(ccache_file):
             ccache_file = os.path.join(ccache_file, 'ccache')
         ...
         return ccache_file

(You don't need to prepend "FILE:", as it is the default ccache type.)

Honza

Dumb me didn't realize that 'ccache_file' is a reference to an actual filesystem path and that the filename can be set dynamically depending on path type (directory vs. file).

Thank you for your comments. Will update the patches accordingly.

--
Martin^3 Babinsky

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to