Code review comment for lp:~lifeless/python-oops-amqp/0.0.2

183 - def __init__(self, config, channel, queue_name):
184 + def __init__(self, config, connection_factory, queue_name):
185 """Create a Receiver.
186
187 :param config: An oops.Config to republish the OOPS reports.
188 - :param channel: An amqplib Channel to listen for reports on.
189 + :param connection: An amqplib connection factory, used to make the
190 + initial connection and to reconnect if that connection is
191 + interrupted.

docstring says the param is "connection", but it's actually "connection_factory".

227 + while not self.stopping and time.time() < self.connection_start + 120:

This is going to stop retrying immediately if the connection dies after more than two minutes.

321 + connection = self.connection_factory()
322 + self.addCleanup(connection.close)
323 + channel = connection.channel()
324 + self.addCleanup(channel.close)
325 + queue = self.useFixture(QueueFixture(channel, self.getUniqueString))

This is now roughly quadruplicated. Put it in a helper or setUp?

318 + def test_publish_inherit_id(self):

Are test_publish and test_publish_inherit_id around the wrong way?

304 + # Publication returns the oops ID allocated.

s/allocated/provided/?

371 - def test_receive_one(self):

Where'd that go?

398 + connection = self.connection_factory()
399 + self.addCleanup(connection.close)
400 + channel = connection.channel()
401 + self.addCleanup(channel.close)
402 + queue = self.useFixture(QueueFixture(channel, self.getUniqueString))

More duplication.

410 + receiver.sentinel = sentinel
411 + receiver.run_forever()
412 + self.assertEqual([expected_report], reports)

May be worth a comment around run_forever to say that it will return quickly since you've already sent the sentinel.

422 + connection = self.connection_factory()
423 + self.addCleanup(connection.close)
424 + channel = connection.channel()
425 + self.addCleanup(channel.close)
426 + queue = self.useFixture(QueueFixture(channel, self.getUniqueString))

Guess what...

499 + # publisher tests is what le aks through when rabbit is shutdown).

You've leaked some whitespace there.

507 + connection = self.connection_factory()
508 + self.addCleanup(connection.close)
509 + channel = connection.channel()
510 + self.addCleanup(channel.close)
511 + queue = self.useFixture(QueueFixture(channel, self.getUniqueString))

Ahem.