Skip to content

Commit

Permalink
Use the --queue-name argument for the consume API
Browse files Browse the repository at this point in the history
Previously, the ``fedora-messaging consume`` CLI only worked if the
"queues" setting was defined. This makes it cumbersome to use.

This sets up a "queues" configuration if the queue name is specified and
passes it on to the API.

Signed-off-by: Jeremy Cline <jcline@redhat.com>
  • Loading branch information
jeremycline authored and mergify[bot] committed Dec 7, 2018
1 parent 2de9676 commit 5019eb0
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 15 deletions.
12 changes: 11 additions & 1 deletion fedora_messaging/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,22 @@ def cli(conf):
@click.option("--exchange", help=_exchange_help)
def consume(exchange, queue_name, routing_key, callback, app_name):
"""Consume messages from an AMQP queue using a Python callback."""
if queue_name:
queues = {
queue_name: {
"durable": True,
"auto_delete": False,
"exclusive": False,
"arguments": {},
}
}
if exchange and queue_name and routing_key:
bindings = [
{"exchange": exchange, "queue": queue_name, "routing_keys": routing_key}
]
elif not exchange and not queue_name and not routing_key:
bindings = config.conf["bindings"]
queues = config.conf["queues"]
else:
raise click.ClickException(
"You must define all three of exchange, queue_name and"
Expand Down Expand Up @@ -140,7 +150,7 @@ def consume(exchange, queue_name, routing_key, callback, app_name):

_log.info("Starting consumer with %s callback", callback_path)
try:
return api.consume(callback, bindings)
return api.consume(callback, bindings=bindings, queues=queues)
except ValueError as e:
click_version = pkg_resources.get_distribution("click").parsed_version
if click_version < pkg_resources.parse_version("7.0"):
Expand Down
52 changes: 38 additions & 14 deletions fedora_messaging/tests/unit/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ def test_good_conf(self, mock_consume):
"""Assert providing a configuration file via the CLI works."""
result = self.runner.invoke(cli.cli, ["--conf=" + GOOD_CONF, "consume"])
mock_consume.assert_called_with(
echo, [{"exchange": "e", "queue": "q", "routing_keys": ["#"]}]
echo,
bindings=[{"exchange": "e", "queue": "q", "routing_keys": ["#"]}],
queues=config.conf["queues"],
)
self.assertEqual(0, result.exit_code)

Expand All @@ -66,7 +68,9 @@ def test_conf_env_support(self, mock_consume):
cli.cli, ["consume"], env={"FEDORA_MESSAGING_CONF": GOOD_CONF}
)
mock_consume.assert_called_with(
echo, [{"exchange": "e", "queue": "q", "routing_keys": ["#"]}]
echo,
bindings=[{"exchange": "e", "queue": "q", "routing_keys": ["#"]}],
queues=config.conf["queues"],
)
self.assertEqual(0, result.exit_code)

Expand Down Expand Up @@ -117,13 +121,21 @@ def test_good_cli_bindings(self, mock_consume, mock_importlib):
mock_importlib.import_module.called_once_with("mod")
mock_consume.assert_called_once_with(
mock_mod_with_callable.callable,
[
bindings=[
{
"exchange": cli_options["exchange"],
"queue": cli_options["queue-name"],
"routing_keys": cli_options["routing-keys"],
}
],
queues={
"qn": {
"durable": True,
"auto_delete": False,
"exclusive": False,
"arguments": {},
}
},
)
self.assertEqual(0, result.exit_code)

Expand All @@ -138,7 +150,9 @@ def test_no_cli_bindings(self, mock_consume, mock_importlib):
mock_importlib.import_module.return_value = mock_mod_with_callable
result = self.runner.invoke(cli.cli, ["consume"])
mock_importlib.import_module.called_once_with("mod")
mock_consume.assert_called_once_with(mock_mod_with_callable.callable, "b")
mock_consume.assert_called_once_with(
mock_mod_with_callable.callable, bindings="b", queues=config.conf["queues"]
)
self.assertEqual(0, result.exit_code)

@mock.patch.dict(
Expand Down Expand Up @@ -187,7 +201,7 @@ def test_missing_cli_and_conf_bindings(self, mock_consume, mock_importlib):
)
self.assertEqual(1, result.exit_code)

@mock.patch.dict("fedora_messaging.config.conf", {"bindings": "b"})
@mock.patch.dict("fedora_messaging.config.conf", {"bindings": "b", "queues": "c"})
@mock.patch("fedora_messaging.cli.importlib")
@mock.patch("fedora_messaging.cli.api.consume")
def test_good_cli_callable(self, mock_consume, mock_importlib):
Expand All @@ -199,10 +213,12 @@ def test_good_cli_callable(self, mock_consume, mock_importlib):
cli.cli, ["consume", "--callback=" + cli_options["callback"]]
)
mock_importlib.import_module.called_once_with("mod")
mock_consume.assert_called_once_with(mock_mod_with_callable.callable, "b")
mock_consume.assert_called_once_with(
mock_mod_with_callable.callable, bindings="b", queues="c"
)
self.assertEqual(0, result.exit_code)

@mock.patch.dict("fedora_messaging.config.conf", {"bindings": "b"})
@mock.patch.dict("fedora_messaging.config.conf", {"bindings": "b", "queues": "c"})
@mock.patch("fedora_messaging.cli.importlib")
@mock.patch("fedora_messaging.cli.api.consume")
def test_app_name(self, mock_consume, mock_importlib, *args, **kwargs):
Expand All @@ -222,7 +238,9 @@ def test_app_name(self, mock_consume, mock_importlib, *args, **kwargs):
config.conf["client_properties"]["app"], cli_options["app-name"]
)
mock_importlib.import_module.called_once_with("mod")
mock_consume.assert_called_once_with(mock_mod_with_callable.callable, "b")
mock_consume.assert_called_once_with(
mock_mod_with_callable.callable, bindings="b", queues="c"
)
self.assertEqual(0, result.exit_code)

@mock.patch.dict(
Expand Down Expand Up @@ -308,7 +326,7 @@ def test_callable_getattr_failure(self, mock_consume, mock_importlib, mock_getat
)
self.assertEqual(1, result.exit_code)

@mock.patch.dict("fedora_messaging.config.conf", {"bindings": "b"})
@mock.patch.dict("fedora_messaging.config.conf", {"bindings": "b", "queues": "q"})
@mock.patch("fedora_messaging.cli.importlib")
@mock.patch("fedora_messaging.cli.api.consume")
def test_consume_improper_callback_object(self, mock_consume, mock_importlib):
Expand All @@ -324,11 +342,13 @@ def test_consume_improper_callback_object(self, mock_consume, mock_importlib):
cli.cli, ["consume", "--callback=" + cli_options["callback"]]
)
mock_importlib.import_module.called_once_with("mod")
mock_consume.assert_called_once_with(mock_mod_with_callable.callable, "b")
mock_consume.assert_called_once_with(
mock_mod_with_callable.callable, bindings="b", queues="q"
)
self.assertIn(error_message, result.output)
self.assertEqual(2, result.exit_code)

@mock.patch.dict("fedora_messaging.config.conf", {"bindings": "b"})
@mock.patch.dict("fedora_messaging.config.conf", {"bindings": "b", "queues": "q"})
@mock.patch("fedora_messaging.cli.importlib")
@mock.patch("fedora_messaging.cli.api.consume")
def test_consume_halt_with_exitcode(self, mock_consume, mock_importlib):
Expand All @@ -346,15 +366,17 @@ def test_consume_halt_with_exitcode(self, mock_consume, mock_importlib):
cli.cli, ["consume", "--callback=" + cli_options["callback"]]
)
mock_importlib.import_module.called_once_with("mod")
mock_consume.assert_called_once_with(mock_mod_with_callable.callable, "b")
mock_consume.assert_called_once_with(
mock_mod_with_callable.callable, bindings="b", queues="q"
)
mock_log.error.assert_called_once_with(
"Consumer halted with non-zero exit code (%d): %s",
5,
"User halted execution",
)
self.assertEqual(halt_exit_code, result.exit_code)

@mock.patch.dict("fedora_messaging.config.conf", {"bindings": "b"})
@mock.patch.dict("fedora_messaging.config.conf", {"bindings": "b", "queues": "q"})
@mock.patch("fedora_messaging.cli.importlib")
@mock.patch("fedora_messaging.cli.api.consume")
def test_consume_halt_without_exitcode(self, mock_consume, mock_importlib):
Expand All @@ -367,5 +389,7 @@ def test_consume_halt_without_exitcode(self, mock_consume, mock_importlib):
cli.cli, ["consume", "--callback=" + cli_options["callback"]]
)
mock_importlib.import_module.called_once_with("mod")
mock_consume.assert_called_once_with(mock_mod_with_callable.callable, "b")
mock_consume.assert_called_once_with(
mock_mod_with_callable.callable, bindings="b", queues="q"
)
self.assertEqual(0, result.exit_code)

0 comments on commit 5019eb0

Please sign in to comment.