launchpad-reviewers team mailing list archive

[Robert Collins <robertc@xxxxxxxxxxxxxxxxx>]

Robert Collins has proposed merging lp:~lifeless/python-oops-amqp/bug-901497 into lp:python-oops-amqp.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #901497 in python-oops-amqp: "close_ignoring_EPIPE can trigger IOError"
  https://bugs.launchpad.net/python-oops-amqp/+bug/901497

For more details, see:
https://code.launchpad.net/~lifeless/python-oops-amqp/bug-901497/+merge/84915

Fix some more resiliency issues detected during the recent rabbit meltdown.
-- 
https://code.launchpad.net/~lifeless/python-oops-amqp/bug-901497/+merge/84915
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/python-oops-amqp/bug-901497 into lp:python-oops-amqp.

=== modified file 'NEWS'
--- NEWS	2011-12-08 09:29:35 +0000
+++ NEWS	2011-12-08 10:18:27 +0000
@@ -8,6 +8,9 @@
 
 * Relicense under LGPL-3. (Robert Collins)
 
+* Catch a wider range of errors when calling close() on channels: they may be
+  in a disconnected/pseudo closed state already. (Robert Collins, #901497)
+
 0.0.4
 -----
 

=== modified file 'oops_amqp/receiver.py'
--- oops_amqp/receiver.py	2011-12-08 09:29:35 +0000
+++ oops_amqp/receiver.py	2011-12-08 10:18:27 +0000
@@ -23,7 +23,7 @@
 
 from utils import (
     amqplib_error_types,
-    close_ignoring_EPIPE,
+    close_ignoring_connection_errors,
     is_amqplib_connection_error,
     )
 
@@ -111,6 +111,6 @@
                     if self.channel.is_open:
                         self.channel.basic_cancel(self.consume_tag)
             finally:
-                close_ignoring_EPIPE(self.channel)
+                close_ignoring_connection_errors(self.channel)
         finally:
-            close_ignoring_EPIPE(self.connection)
+            close_ignoring_connection_errors(self.connection)

=== modified file 'oops_amqp/tests/__init__.py'
--- oops_amqp/tests/__init__.py	2011-12-08 09:29:35 +0000
+++ oops_amqp/tests/__init__.py	2011-12-08 10:18:27 +0000
@@ -27,7 +27,7 @@
     ResourcedTestCase,
     )
 
-from oops_amqp.utils import close_ignoring_EPIPE
+from oops_amqp.utils import close_ignoring_connection_errors
 
 __all__ = [
     'ChannelFixture',
@@ -94,9 +94,9 @@
     def setUp(self):
         super(ChannelFixture, self).setUp()
         self.connection = self.connection_factory()
-        self.addCleanup(close_ignoring_EPIPE, self.connection)
+        self.addCleanup(close_ignoring_connection_errors, self.connection)
         self.channel = self.connection.channel()
-        self.addCleanup(close_ignoring_EPIPE, self.channel)
+        self.addCleanup(close_ignoring_connection_errors, self.channel)
 
 
 class TestCase(testtools.TestCase, ResourcedTestCase):

=== modified file 'oops_amqp/tests/test_publisher.py'
--- oops_amqp/tests/test_publisher.py	2011-12-08 09:29:35 +0000
+++ oops_amqp/tests/test_publisher.py	2011-12-08 10:18:27 +0000
@@ -20,7 +20,7 @@
 import bson
 
 from oops_amqp import Publisher
-from oops_amqp.utils import close_ignoring_EPIPE
+from oops_amqp.utils import close_ignoring_connection_errors
 from oops_amqp.tests import (
     ChannelFixture,
     QueueFixture,

=== modified file 'oops_amqp/utils.py'
--- oops_amqp/utils.py	2011-12-08 09:29:35 +0000
+++ oops_amqp/utils.py	2011-12-08 10:18:27 +0000
@@ -22,7 +22,7 @@
 
 __all__ = [
     'amqplib_error_types',
-    'close_ignoring_EPIPE',
+    'close_ignoring_connection_errors',
     'is_amqplib_connection_error',
     'is_amqplib_ioerror',
     ]
@@ -38,12 +38,13 @@
 amqplib_error_types = amqplib_connection_errors + (IOError,)
 
 
-def close_ignoring_EPIPE(closable):
+def close_ignoring_connection_errors(closable):
     try:
         return closable.close()
-    except socket.error, e:
-        if e.errno != errno.EPIPE:
-            raise
+    except amqplib_error_types, e:
+        if is_amqplib_connection_error(e):
+            return
+        raise
 
 
 def is_amqplib_ioerror(e):