Dne 16.3.2015 v 13:30 Martin Babinsky napsal(a):
On 03/16/2015 12:15 PM, Martin Kosek wrote:
On 03/13/2015 05:37 PM, Martin Babinsky wrote:
Attaching the next iteration of patches.

I have tried my best to reword the ipa-client-install man page bit
about the
new option. Any suggestions to further improve it are welcome.

I have also slightly modified the 'kinit_keytab' function so that in
Kerberos
errors are reported for each attempt and the text of the last error
is retained
when finally raising exception.

The approach looks very good. I think that my only concern with this
patch is
this part:

+            ccache.init_creds_keytab(keytab=ktab, principal=princ)
...
+        except krbV.Krb5Error as e:
+            last_exc = str(e)
+            root_logger.debug("Attempt %d/%d: failed: %s"
+                              % (attempt, attempts, last_exc))
+            time.sleep(1)
+
+    root_logger.debug("Maximum number of attempts (%d) reached"
+                      % attempts)
+    raise StandardError("Error initializing principal %s: %s"
+                        % (principal, last_exc))

The problem here is that this function will raise the super-generic
StandardError instead of the proper with all the context and
information about
the error that the caller can then process.

I think that

     except krbV.Krb5Error as e:
         if attempt == max_attempts:
             log something
             raise

would be better.


Yes that seems reasonable. I'm just thinking whether we should re-raise
Krb5Error or raise ipalib.errors.KerberosError? the latter options makes
more sense to me as we would not have to additionally import Krb5Error
everywhere and it would also make the resulting errors more consistent.

I am thinking about someting like this:

     except krbV.Krb5Error as e:
        if attempt == attempts:
            # log that we have reaches maximum number of attempts
            raise KerberosError(minor=str(e))

What do you think?


NACK, don't use ipalib from ipapython in new code, we are trying to get rid of this circular dependency. Krb5Error is OK in this case.

--
Jan Cholasta

--
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