diff --git a/fedora_messaging/cli.py b/fedora_messaging/cli.py index 51f5fbbf..44ddc94c 100644 --- a/fedora_messaging/cli.py +++ b/fedora_messaging/cli.py @@ -51,6 +51,13 @@ "and should point to either a function or a class. Consult the API " "documentation for the interface required for these objects." ) +_callback_file_help = ( + "The path of a Python file that contains the callable object to" + " execute when the message arrives. This should be in the format " + '"my/script.py:object_in_file". Providing this overrides the callback set ' + 'by the "--callback" argument or configuration file. It executes the entire ' + "Python file to load the callable object." +) _routing_key_help = ( "The AMQP routing key to use with the queue. This controls what messages are " "delivered to the consumer. Can be specified multiple times; any message " @@ -91,11 +98,12 @@ def cli(conf): @cli.command() @click.option("--app-name", help=_app_name_help) +@click.option("--callback-file", help=_callback_file_help) @click.option("--callback", help=_callback_help) @click.option("--routing-key", help=_routing_key_help, multiple=True) @click.option("--queue-name", help=_queue_name_help) @click.option("--exchange", help=_exchange_help) -def consume(exchange, queue_name, routing_key, callback, app_name): +def consume(exchange, queue_name, routing_key, callback, callback_file, app_name): """Consume messages from an AMQP queue using a Python callback.""" # The configuration validates these are not null and contain all required keys # when it is loaded. @@ -123,6 +131,94 @@ def consume(exchange, queue_name, routing_key, callback, app_name): for binding in bindings: binding["routing_keys"] = routing_key + if callback_file: + callback = _callback_from_filesystem(callback_file) + else: + callback = _callback_from_python_path(callback) + + if app_name: + config.conf["client_properties"]["app"] = app_name + + try: + deferred_consumers = api.twisted_consume( + callback, bindings=bindings, queues=queues + ) + deferred_consumers.addCallback(_consume_callback) + deferred_consumers.addErrback(_consume_errback) + except ValueError as e: + click_version = pkg_resources.get_distribution("click").parsed_version + if click_version < pkg_resources.parse_version("7.0"): + raise click.exceptions.BadOptionUsage(str(e)) + else: + raise click.exceptions.BadOptionUsage("callback", str(e)) + + reactor.run() + sys.exit(_exit_code) + + +def _callback_from_filesystem(callback_file): + """ + Load a callable from a Python script on the file system. + + Args: + callback_file (str): The callback as a filesystem path and callable name + separated by a ":". For example, "my/python/file.py:printer". + + Raises: + click.ClickException: If the object cannot be loaded. + + Returns: + callable: The callable object. + """ + try: + file_path, callable_name = callback_file.strip().split(":") + except ValueError: + raise click.ClickException( + "Unable to parse the '--callback-file' option; the " + 'expected format is "path/to/file.py:callable_object" where ' + '"callable_object" is the name of the function or class in the ' + "Python file" + ) + + try: + file_namespace = {} + with open(file_path, "rb") as fd: + try: + # Using "exec" is generally a Bad Idea (TM), so bandit is upset at + # us. In this case, it seems like a Good Idea (TM), but I might be + # wrong. Sorry. + exec(compile(fd.read(), file_path, "exec"), file_namespace) # nosec + except Exception as e: + raise click.ClickException( + "The {} file raised the following exception during execution:" + " {}".format(file_path, str(e)) + ) + + if callable_name not in file_namespace: + err = "The '{}' object was not found in the '{}' file.".format( + callable_name, file_path + ) + raise click.ClickException(err) + else: + return file_namespace[callable_name] + except IOError as e: + raise click.ClickException("An IO error occurred: {}".format(str(e))) + + +def _callback_from_python_path(callback): + """ + Load a callable from a Python path. + + Args: + callback_file (str): The callback as a Python path and callable name + separated by a ":". For example, "my_package.my_module:printer". + + Raises: + click.ClickException: If the object cannot be loaded. + + Returns: + callable: The callable object. + """ callback_path = callback or config.conf["callback"] if not callback_path: raise click.ClickException( @@ -148,7 +244,7 @@ def consume(exchange, queue_name, routing_key, callback, app_name): ) try: - callback = getattr(module, cls) + callback_object = getattr(module, cls) except AttributeError as e: raise click.ClickException( "Unable to import {} ({}); is the package installed? The python path should " @@ -156,26 +252,8 @@ def consume(exchange, queue_name, routing_key, callback, app_name): callback_path, str(e) ) ) - - if app_name: - config.conf["client_properties"]["app"] = app_name - _log.info("Starting consumer with %s callback", callback_path) - try: - deferred_consumers = api.twisted_consume( - callback, bindings=bindings, queues=queues - ) - deferred_consumers.addCallback(_consume_callback) - deferred_consumers.addErrback(_consume_errback) - except ValueError as e: - click_version = pkg_resources.get_distribution("click").parsed_version - if click_version < pkg_resources.parse_version("7.0"): - raise click.exceptions.BadOptionUsage(str(e)) - else: - raise click.exceptions.BadOptionUsage("callback", str(e)) - - reactor.run() - sys.exit(_exit_code) + return callback_object def _consume_errback(failure): diff --git a/fedora_messaging/tests/fixtures/bad_cb b/fedora_messaging/tests/fixtures/bad_cb new file mode 100644 index 00000000..0e1dc8ba --- /dev/null +++ b/fedora_messaging/tests/fixtures/bad_cb @@ -0,0 +1 @@ +not a valid python file diff --git a/fedora_messaging/tests/fixtures/callback.py b/fedora_messaging/tests/fixtures/callback.py new file mode 100644 index 00000000..f774a1fa --- /dev/null +++ b/fedora_messaging/tests/fixtures/callback.py @@ -0,0 +1,2 @@ +def rand(message): + return 4 diff --git a/fedora_messaging/tests/unit/test_cli.py b/fedora_messaging/tests/unit/test_cli.py index 810a2d61..d575cd82 100644 --- a/fedora_messaging/tests/unit/test_cli.py +++ b/fedora_messaging/tests/unit/test_cli.py @@ -23,6 +23,7 @@ from click.testing import CliRunner from twisted.internet import error from twisted.python import failure +import click import mock from fedora_messaging import cli, config, exceptions @@ -444,3 +445,65 @@ def test_errback_general_exception(self, mock_reactor): cli._consume_errback(f) self.assertEqual(11, cli._exit_code) + + +class CallbackFromFilesytem(unittest.TestCase): + """Unit tests for :func:`fedora_messaging.cli._callback_from_filesystem`.""" + + def test_good_callback(self): + """Assert loading a callback from a file works.""" + cb = cli._callback_from_filesystem( + os.path.join(FIXTURES_DIR, "callback.py") + ":rand" + ) + self.assertEqual(4, cb(None)) + + def test_bad_format(self): + """Assert an exception is raised if the format is bad.""" + with self.assertRaises(click.ClickException) as cm: + cli._callback_from_filesystem("file/with/no/function.py") + + self.assertEqual( + "Unable to parse the '--callback-file' option; the " + 'expected format is "path/to/file.py:callable_object" where ' + '"callable_object" is the name of the function or class in the ' + "Python file", + cm.exception.message, + ) + + def test_invalid_file(self): + """Assert an exception is raised if the Python file can't be executed.""" + with self.assertRaises(click.ClickException) as cm: + cli._callback_from_filesystem( + os.path.join(FIXTURES_DIR, "bad_cb") + ":missing" + ) + + self.assertEqual( + "The {} file raised the following exception during execution: " + "invalid syntax (bad_cb, line 1)".format( + os.path.join(FIXTURES_DIR, "bad_cb") + ), + cm.exception.message, + ) + + def test_callable_does_not_exist(self): + """Assert an exception is raised if the callable is missing.""" + with self.assertRaises(click.ClickException) as cm: + cli._callback_from_filesystem( + os.path.join(FIXTURES_DIR, "callback.py") + ":missing" + ) + + self.assertEqual( + "The 'missing' object was not found in the '{}' file." + "".format(os.path.join(FIXTURES_DIR, "callback.py")), + cm.exception.message, + ) + + def test_file_does_not_exist(self): + """Assert an exception is raised if the file doesn't exist.""" + with self.assertRaises(click.ClickException) as cm: + cli._callback_from_filesystem("file/that/is/missing.py:callable") + + self.assertEqual( + "An IO error occurred: [Errno 2] No such file or directory: 'file/that/is/missing.py'", + cm.exception.message, + ) diff --git a/news/159.feature b/news/159.feature new file mode 100644 index 00000000..cdab3c29 --- /dev/null +++ b/news/159.feature @@ -0,0 +1,3 @@ +"fedora-messaging consume" now accepts a "--callback-file" argument which will +load a callback function from an arbitrary Python file. Previously, it was +required that the callback be in the Python path.