On Wed, 15 Jan 2014, Jan Cholasta wrote:
On 15.1.2014 13:21, Alexander Bokovoy wrote:
On Wed, 15 Jan 2014, Jan Cholasta wrote:
On 15.1.2014 12:17, Martin Kosek wrote:
On 01/15/2014 11:20 AM, Alexander Bokovoy wrote:
On Wed, 15 Jan 2014, Jan Cholasta wrote:
Hi,

the attached patch should fix
<https://fedorahosted.org/freeipa/ticket/4078>.

I have also attached patch 179, which fixes a related bug in
certificate
renewal.

NACK for this part:
This fixes a possible NSS database corruption in renew_ca_cert.
---
ipaserver/install/installutils.py | 3 ---
1 file changed, 3 deletions(-)

diff --git a/ipaserver/install/installutils.py
b/ipaserver/install/installutils.py
index 67eabc2..0ba9c2e 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -820,9 +820,6 @@ def stopped_service(service, instance_name=""):
       root_logger.debug('Service %s%s is not running, continue.',
service,
                         log_instance_name)
       yield
-        root_logger.debug('Starting %s%s.', service,
log_instance_name)
-        ipaservices.knownservices[service].start(instance_name)
-        return
   else:
       # Stop the service, do the required stuff and start it again
       root_logger.debug('Stopping %s%s.', service,
log_instance_name)
You need to wrap yield into try: finally: block. I have a patch for
similar case in private_cache() few lines above this code.

There is no cleanup needed for this particular yield. Also this is not
really the concern of the patch, it does exactly what the commit
message says. Since you are already fixing a similar case, I would
suggest you amend your patch with any changes you deem necessary, I
don't see why a single fix should be dispersed among multiple patches.
Patch attached, it obsoletes your patch 179.

Thanks, but I don't understand why you squashed my patch 179 into your patch, the fixes are for separate issues (yield exception handling vs. previously stopped service being started).
Because you just said above:
suggest you amend your patch with any changes you deem necessary, I
don't see why a single fix should be dispersed among multiple patches.
"a single fix" is now not dispersed among multiple patches.

--
/ Alexander Bokovoy

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to