Skip to content

Commit

Permalink
Don't declare exchanges when consuming
Browse files Browse the repository at this point in the history
The CLI (by way of Twisted) does not declare exchanges when consuming,
it assumes the exchange already exists. This seems reasonable, consumers
will still learn about configuration mistakes during the binding
process. This makes it impossible to consume from the public vhost in
Fedora, so this is a quick bugfix release while the transition to
Crochet and Twisted are worked on and polished.

Fixes: #171

Signed-off-by: Jeremy Cline <jcline@redhat.com>
  • Loading branch information
jeremycline authored and mergify[bot] committed Jun 24, 2019
1 parent af2a56d commit d43c261
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 61 deletions.
24 changes: 0 additions & 24 deletions fedora_messaging/_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -240,16 +240,6 @@ def _on_qosok(self, qosok_frame):
Args:
qosok_frame (pika.spec.Basic.Qos): The frame send from the server.
"""
for name, args in self._exchanges.items():
self._channel.exchange_declare(
exchange=name,
exchange_type=args["type"],
durable=args["durable"],
auto_delete=args["auto_delete"],
arguments=args["arguments"],
passive=config.conf["passive_declares"],
callback=self._on_exchange_declareok,
)
for name, args in self._queues.items():
self._channel.queue_declare(
queue=name,
Expand Down Expand Up @@ -346,19 +336,6 @@ def _on_connection_error(self, connection, error_message):
_log.error(error_message)
self.call_later(1, self.reconnect) # TODO: exponential backoff?

def _on_exchange_declareok(self, declare_frame):
"""
Callback invoked when an exchange is successfully declared.
It will declare the queues in the bindings dictionary with the
:meth:`_on_queue_declareok` callback.
Args:
frame (pika.spec.Exchange.DeclareOk): The DeclareOk frame from the
server.
"""
_log.info("Exchange declared successfully")

def _on_queue_declareok(self, frame):
"""
Callback invoked when a queue is successfully declared.
Expand Down Expand Up @@ -462,7 +439,6 @@ def consume(self, callback, bindings=None, queues=None, exchanges=None):
"""
self._bindings = bindings or config.conf["bindings"]
self._queues = queues or config.conf["queues"]
self._exchanges = exchanges or config.conf["exchanges"]

# If the callback is a class, create an instance of it first
if inspect.isclass(callback):
Expand Down
37 changes: 0 additions & 37 deletions fedora_messaging/tests/unit/test_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -361,15 +361,13 @@ def callback(m):
self.consumer.consume(callback)
self.assertEqual(self.consumer._bindings, config.conf["bindings"])
self.assertEqual(self.consumer._queues, config.conf["queues"])
self.assertEqual(self.consumer._exchanges, config.conf["exchanges"])
# Configuration overrides
test_value = [{"test": "test"}]
self.consumer.consume(
callback, bindings=test_value, queues=test_value, exchanges=test_value
)
self.assertEqual(self.consumer._bindings, test_value)
self.assertEqual(self.consumer._queues, test_value)
self.assertEqual(self.consumer._exchanges, test_value)

def test_consume_uncallable_callback(self):
"""Test the consume function with not callable callback."""
Expand Down Expand Up @@ -414,28 +412,18 @@ def stop_consumer():
self.consumer.consume(callback)
self.assertEqual(self.consumer._bindings, config.conf["bindings"])
self.assertEqual(self.consumer._queues, config.conf["queues"])
self.assertEqual(self.consumer._exchanges, config.conf["exchanges"])
# Configuration overrides
test_value = [{"test": "test"}]
self.consumer.consume(
callback, bindings=test_value, queues=test_value, exchanges=test_value
)
self.assertEqual(self.consumer._bindings, test_value)
self.assertEqual(self.consumer._queues, test_value)
self.assertEqual(self.consumer._exchanges, test_value)

def test_declare(self):
# Test that the exchanges, queues and bindings are properly
# declared.
self.consumer._channel = mock.Mock()
self.consumer._exchanges = {
"testexchange": {
"type": "type",
"durable": "durable",
"auto_delete": "auto_delete",
"arguments": "arguments",
}
}
self.consumer._queues = {
"testqueue": {
"durable": "durable",
Expand All @@ -453,15 +441,6 @@ def test_declare(self):
]
# Declare exchanges and queues
self.consumer._on_qosok(None)
self.consumer._channel.exchange_declare.assert_called_with(
exchange="testexchange",
exchange_type="type",
durable="durable",
auto_delete="auto_delete",
arguments="arguments",
passive=False,
callback=self.consumer._on_exchange_declareok,
)
self.consumer._channel.queue_declare.assert_called_with(
queue="testqueue",
durable="durable",
Expand Down Expand Up @@ -489,14 +468,6 @@ def test_declare_passive(self):
# Test that the exchanges, queues and bindings are passively declared
# if configured so.
self.consumer._channel = mock.Mock()
self.consumer._exchanges = {
"testexchange": {
"type": "type",
"durable": "durable",
"auto_delete": "auto_delete",
"arguments": "arguments",
}
}
self.consumer._queues = {
"testqueue": {
"durable": "durable",
Expand All @@ -508,8 +479,6 @@ def test_declare_passive(self):
with mock.patch.dict(config.conf, {"passive_declares": True}):
# Declare exchanges and queues
self.consumer._on_qosok(None)
call_args = self.consumer._channel.exchange_declare.call_args_list[-1][1]
assert call_args.get("passive") is True
call_args = self.consumer._channel.queue_declare.call_args_list[-1][1]
assert call_args.get("passive") is True

Expand Down Expand Up @@ -586,12 +555,6 @@ def test_on_cancel(self):
self.consumer._on_cancel("cancel_frame")
mock_log.info.assert_called_once_with("Server canceled consumer")

def test_on_exchange_declareok(self):
"""Assert proper information is logged on callback _on_exchange_declareok call."""
with mock.patch("fedora_messaging._session._log") as mock_log:
self.consumer._on_exchange_declareok("declare_frame")
mock_log.info.assert_called_once_with("Exchange declared successfully")

def test_connection_error_with_string_as_error(self):
"""Assert callback is called on connection error."""
connection = "test_connection"
Expand Down

0 comments on commit d43c261

Please sign in to comment.