Jan Cholasta wrote: > Dne 28.7.2014 v 21:39 Rob Crittenden napsal(a): >> This is oh-so close. AFAICT it generally does what it should, I think it >> is ready for a wider audience. Just a few more things: >> >> 306: A while True loop is used for something which AFAICT can only ever >> execute once. I'd think something like this is more readable: >> >> for ca_nick, ca_flags in db.list_certs(): >> if db.has_nickname(ca_cert): >> try: >> db.delete_cert(ca_nick) >> except ipautil.CalledProcessError: >> syslog.syslog( >> syslog.LOG_ERR, >> "Failed to remove certificate %s" % ca_nick) > > Actually the while loop is necessary, because certutil -D (and in turn > CertDB.delete_cert) deletes just a single cert with the nickname, but > there may be more versions of it and we need to delete them all.
Aha, excellent point. This would be a great comment in the code! >> >> +1 on the additional syslogs. It will help figure out what's going on if >> things go sideways. >> >> Otherwise things seem to be working. I think that fixing the above is >> enough for a +57 with the promise of unit tests to back up some of these >> new functions. > > I'm working on that. > >> >> rob >> rob >> > > I have made a slight adjustment to patch 246 because of > <https://fedorahosted.org/freeipa/ticket/4039>, see > <http://www.redhat.com/archives/freeipa-devel/2014-July/msg00369.html>. > > Updated rebased patches attached. > > (once again, the correct order to apply them is 241-253, 262-294, > 303-305, 295-299, 306-307) > ACK on 246. IMHO this is ready to be pushed if you can add the comment on 306. A slight rebase on 251 is needed for freeipa.spec.in. rob _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel