diff --git a/packaged_helloworld/README.md b/packaged_helloworld/README.md new file mode 100644 index 000000000..2a2fbfacb --- /dev/null +++ b/packaged_helloworld/README.md @@ -0,0 +1,2 @@ +This is a boilerplate package for a Zulip bot that can be installed from pip +and launched using the `zulip-run-bots` command. diff --git a/packaged_helloworld/packaged_helloworld/__init__.py b/packaged_helloworld/packaged_helloworld/__init__.py new file mode 100644 index 000000000..5becc17c0 --- /dev/null +++ b/packaged_helloworld/packaged_helloworld/__init__.py @@ -0,0 +1 @@ +__version__ = "1.0.0" diff --git a/packaged_helloworld/packaged_helloworld/doc.md b/packaged_helloworld/packaged_helloworld/doc.md new file mode 100644 index 000000000..864e8805d --- /dev/null +++ b/packaged_helloworld/packaged_helloworld/doc.md @@ -0,0 +1,5 @@ +Simple Zulip bot that will respond to any query with a "beep boop". + +The packaged_helloworld bot is a boilerplate bot that can be used as a +template for more sophisticated/evolved Zulip bots that can be +installed separately. diff --git a/packaged_helloworld/packaged_helloworld/packaged_helloworld.py b/packaged_helloworld/packaged_helloworld/packaged_helloworld.py new file mode 100644 index 000000000..a8b9aefb8 --- /dev/null +++ b/packaged_helloworld/packaged_helloworld/packaged_helloworld.py @@ -0,0 +1,30 @@ +# See readme.md for instructions on running this code. +from typing import Any, Dict + +import packaged_helloworld + +from zulip_bots.lib import BotHandler + +__version__ = packaged_helloworld.__version__ + + +class HelloWorldHandler: + def usage(self) -> str: + return """ + This is a boilerplate bot that responds to a user query with + "beep boop", which is robot for "Hello World". + + This bot can be used as a template for other, more + sophisticated, bots that can be installed separately. + """ + + def handle_message(self, message: Dict[str, Any], bot_handler: BotHandler) -> None: + content = "beep boop" # type: str + bot_handler.send_reply(message, content) + + emoji_name = "wave" # type: str + bot_handler.react(message, emoji_name) + return + + +handler_class = HelloWorldHandler diff --git a/packaged_helloworld/setup.py b/packaged_helloworld/setup.py new file mode 100644 index 000000000..d8eebe1ea --- /dev/null +++ b/packaged_helloworld/setup.py @@ -0,0 +1,13 @@ +import packaged_helloworld +from setuptools import find_packages, setup + +package_info = { + "name": "packaged_helloworld", + "version": packaged_helloworld.__version__, + "entry_points": { + "zulip_bots.registry": ["packaged_helloworld=packaged_helloworld.packaged_helloworld"], + }, + "packages": find_packages(), +} + +setup(**package_info) diff --git a/zulip_bots/setup.py b/zulip_bots/setup.py index 9ab6ca163..08acc4331 100644 --- a/zulip_bots/setup.py +++ b/zulip_bots/setup.py @@ -66,6 +66,7 @@ "lxml", "BeautifulSoup4", "typing_extensions", + 'importlib-metadata >= 3.6; python_version < "3.10"', ], ) diff --git a/zulip_bots/zulip_bots/finder.py b/zulip_bots/zulip_bots/finder.py index 354faf829..0f8ee885c 100644 --- a/zulip_bots/zulip_bots/finder.py +++ b/zulip_bots/zulip_bots/finder.py @@ -3,10 +3,13 @@ import importlib.util import os from pathlib import Path +from types import ModuleType from typing import Any, Optional, Tuple current_dir = os.path.dirname(os.path.abspath(__file__)) +import importlib_metadata as metadata + def import_module_from_source(path: str, name: str) -> Any: spec = importlib.util.spec_from_file_location(name, path) @@ -25,6 +28,54 @@ def import_module_by_name(name: str) -> Any: return None +class DuplicateRegisteredBotName(Exception): + pass + + +def import_module_from_zulip_bot_registry(name: str) -> Tuple[str, Optional[ModuleType]]: + # Prior to Python 3.10, calling importlib.metadata.entry_points returns a + # SelectableGroups object when no parameters is given. Currently we use + # the importlib_metadata library for compatibility, but we need to migrate + # to the built-in library when we start to adapt Python 3.10. + # https://importlib-metadata.readthedocs.io/en/latest/using.html#entry-points + registered_bots = metadata.entry_points(group="zulip_bots.registry") + matching_bots = [bot for bot in registered_bots if bot.name == name] + + if len(matching_bots) == 1: # Unique matching entrypoint + """We expect external bots to be registered using entry_points in the + group "zulip_bots.registry", where the name of the entry point should + match the name of the module containing the bot handler and the value + of it should be the package containing the bot handler module. + + E.g, an Python package for a bot called "packaged_bot" should have an + `entry_points` setup like the following: + + setup( + ... + entry_points={ + "zulip_bots.registry":[ + "packaged_bot=packaged_bot.packaged_bot" + ] + } + ... + ) + """ + bot = matching_bots[0] + bot_name = bot.name + bot_module = bot.load() + bot_version = bot_module.__version__ + + if bot_version is not None: + return f"{bot_name}: {bot_version}", bot_module + else: + return f"editable package: {bot_name}", bot_module + + if len(matching_bots) > 1: + raise DuplicateRegisteredBotName(name) + + return "", None # no matches in registry + + def resolve_bot_path(name: str) -> Optional[Tuple[Path, str]]: if os.path.isfile(name): bot_path = Path(name) diff --git a/zulip_bots/zulip_bots/lib.py b/zulip_bots/zulip_bots/lib.py index d6d79f7c5..1e526d802 100644 --- a/zulip_bots/zulip_bots/lib.py +++ b/zulip_bots/zulip_bots/lib.py @@ -443,6 +443,7 @@ def run_message_handler_for_bot( config_file: str, bot_config_file: str, bot_name: str, + bot_source: str, ) -> Any: """ lib_module is of type Any, since it can contain any bot's @@ -473,7 +474,7 @@ def run_message_handler_for_bot( message_handler = prepare_message_handler(bot_name, restricted_client, lib_module) if not quiet: - print("Running {} Bot:".format(bot_details["name"])) + print("Running {} Bot (from {}):".format(bot_details["name"], bot_source)) if bot_details["description"] != "": print("\n\t{}".format(bot_details["description"])) if hasattr(message_handler, "usage"): diff --git a/zulip_bots/zulip_bots/run.py b/zulip_bots/zulip_bots/run.py index 6d8bf7607..e26922318 100755 --- a/zulip_bots/zulip_bots/run.py +++ b/zulip_bots/zulip_bots/run.py @@ -48,6 +48,13 @@ def parse_args() -> argparse.Namespace: help="try running the bot even if dependencies install fails", ) + parser.add_argument( + "--registry", + "-r", + action="store_true", + help="run the bot via zulip_bots registry", + ) + parser.add_argument("--provision", action="store_true", help="install dependencies for the bot") args = parser.parse_args() @@ -109,36 +116,50 @@ def exit_gracefully_if_bot_config_file_does_not_exist(bot_config_file: Optional[ def main() -> None: args = parse_args() - result = finder.resolve_bot_path(args.bot) - if result: - bot_path, bot_name = result - sys.path.insert(0, os.path.dirname(bot_path)) - - if args.provision: - provision_bot(os.path.dirname(bot_path), args.force) - + if args.registry: try: - lib_module = finder.import_module_from_source(bot_path.as_posix(), bot_name) - except ImportError: - req_path = os.path.join(os.path.dirname(bot_path), "requirements.txt") - with open(req_path) as fp: - deps_list = fp.read() - - dep_err_msg = ( - "ERROR: The following dependencies for the {bot_name} bot are not installed:\n\n" - "{deps_list}\n" - "If you'd like us to install these dependencies, run:\n" - " zulip-run-bot {bot_name} --provision" + bot_source, lib_module = finder.import_module_from_zulip_bot_registry(args.bot) + except finder.DuplicateRegisteredBotName as error: + print( + f'ERROR: Found duplicate entries for "{error}" in zulip bots registry.\n' + "Make sure that you don't install bots using the same entry point. Exiting now." ) - print(dep_err_msg.format(bot_name=bot_name, deps_list=deps_list)) sys.exit(1) - else: - lib_module = finder.import_module_by_name(args.bot) if lib_module: - bot_name = lib_module.__name__ + bot_name = args.bot + else: + result = finder.resolve_bot_path(args.bot) + if result: + bot_path, bot_name = result + sys.path.insert(0, os.path.dirname(bot_path)) + if args.provision: - print("ERROR: Could not load bot's module for '{}'. Exiting now.") + provision_bot(os.path.dirname(bot_path), args.force) + + try: + lib_module = finder.import_module_from_source(bot_path.as_posix(), bot_name) + except ImportError: + req_path = os.path.join(os.path.dirname(bot_path), "requirements.txt") + with open(req_path) as fp: + deps_list = fp.read() + + dep_err_msg = ( + "ERROR: The following dependencies for the {bot_name} bot are not installed:\n\n" + "{deps_list}\n" + "If you'd like us to install these dependencies, run:\n" + " zulip-run-bot {bot_name} --provision" + ) + print(dep_err_msg.format(bot_name=bot_name, deps_list=deps_list)) sys.exit(1) + bot_source = "source" + else: + lib_module = finder.import_module_by_name(args.bot) + if lib_module: + bot_name = lib_module.__name__ + bot_source = "named module" + if args.provision: + print("ERROR: Could not load bot's module for '{}'. Exiting now.") + sys.exit(1) if lib_module is None: print("ERROR: Could not load bot module. Exiting now.") @@ -160,6 +181,7 @@ def main() -> None: bot_config_file=args.bot_config_file, quiet=args.quiet, bot_name=bot_name, + bot_source=bot_source, ) except NoBotConfigException: print( diff --git a/zulip_bots/zulip_bots/tests/test_lib.py b/zulip_bots/zulip_bots/tests/test_lib.py index e60e77b59..e83ba548e 100644 --- a/zulip_bots/zulip_bots/tests/test_lib.py +++ b/zulip_bots/zulip_bots/tests/test_lib.py @@ -197,6 +197,7 @@ def test_message(message, flags): config_file=None, bot_config_file=None, bot_name="testbot", + bot_source="bot code location", ) def test_upload_file(self): diff --git a/zulip_bots/zulip_bots/tests/test_run.py b/zulip_bots/zulip_bots/tests/test_run.py index 6b233d5d3..a549c30f5 100644 --- a/zulip_bots/zulip_bots/tests/test_run.py +++ b/zulip_bots/zulip_bots/tests/test_run.py @@ -5,9 +5,10 @@ from pathlib import Path from typing import Optional from unittest import TestCase, mock -from unittest.mock import patch +from unittest.mock import MagicMock, patch import zulip_bots.run +from zulip_bots.finder import metadata from zulip_bots.lib import extract_query_without_mention @@ -15,6 +16,10 @@ class TestDefaultArguments(TestCase): our_dir = os.path.dirname(__file__) path_to_bot = os.path.abspath(os.path.join(our_dir, "../bots/giphy/giphy.py")) + packaged_bot_module = MagicMock(__version__="1.0.0") + packaged_bot_entrypoint = metadata.EntryPoint( + "packaged_bot", "module_name", "zulip_bots.registry" + ) @patch("sys.argv", ["zulip-run-bot", "giphy", "--config-file", "/foo/bar/baz.conf"]) @patch("zulip_bots.run.run_message_handler_for_bot") @@ -29,6 +34,7 @@ def test_argument_parsing_with_bot_name( config_file="/foo/bar/baz.conf", bot_config_file=None, lib_module=mock.ANY, + bot_source="source", quiet=False, ) @@ -45,6 +51,32 @@ def test_argument_parsing_with_bot_path( config_file="/foo/bar/baz.conf", bot_config_file=None, lib_module=mock.ANY, + bot_source="source", + quiet=False, + ) + + @patch( + "sys.argv", ["zulip-run-bot", "packaged_bot", "--config-file", "/foo/bar/baz.conf", "-r"] + ) + @patch("zulip_bots.run.run_message_handler_for_bot") + def test_argument_parsing_with_zulip_bot_registry( + self, mock_run_message_handler_for_bot: mock.Mock + ) -> None: + with patch("zulip_bots.run.exit_gracefully_if_zulip_config_is_missing"), patch( + "zulip_bots.finder.metadata.EntryPoint.load", + return_value=self.packaged_bot_module, + ), patch( + "zulip_bots.finder.metadata.entry_points", + return_value=(self.packaged_bot_entrypoint,), + ): + zulip_bots.run.main() + + mock_run_message_handler_for_bot.assert_called_with( + bot_name="packaged_bot", + config_file="/foo/bar/baz.conf", + bot_config_file=None, + lib_module=mock.ANY, + bot_source="packaged_bot: 1.0.0", quiet=False, ) diff --git a/zulip_botserver/tests/server_test_lib.py b/zulip_botserver/tests/server_test_lib.py index fc64d1864..8a1cb500d 100644 --- a/zulip_botserver/tests/server_test_lib.py +++ b/zulip_botserver/tests/server_test_lib.py @@ -29,7 +29,7 @@ def assert_bot_server_response( server.app.config["BOTS_LIB_MODULES"] = bots_lib_modules if bot_handlers is None: bot_handlers = server.load_bot_handlers( - available_bots, bots_config, third_party_bot_conf + available_bots, bots_lib_modules, bots_config, third_party_bot_conf ) message_handlers = server.init_message_handlers( available_bots, bots_lib_modules, bot_handlers diff --git a/zulip_botserver/tests/test_server.py b/zulip_botserver/tests/test_server.py index 1b1b6fe02..59367f760 100644 --- a/zulip_botserver/tests/test_server.py +++ b/zulip_botserver/tests/test_server.py @@ -8,6 +8,7 @@ from typing import Any, Dict from unittest import mock +from zulip_bots.finder import metadata from zulip_bots.lib import BotHandler from zulip_botserver import server from zulip_botserver.input_parameters import parse_args @@ -273,6 +274,37 @@ def test_load_lib_modules(self) -> None: ).as_posix() module = server.load_lib_modules([path])[path] + @mock.patch("zulip_botserver.server.app") + @mock.patch("sys.argv", ["zulip-botserver", "--config-file", "/foo/bar/baz.conf"]) + def test_load_from_registry(self, mock_app: mock.Mock) -> None: + packaged_bot_module = mock.MagicMock(__version__="1.0.0", __file__="asd") + packaged_bot_entrypoint = metadata.EntryPoint( + "packaged_bot", "module_name", "zulip_bots.registry" + ) + bots_config = { + "packaged_bot": { + "email": "packaged-bot@zulip.com", + "key": "value", + "site": "http://localhost", + "token": "abcd1234", + } + } + + with mock.patch( + "zulip_botserver.server.read_config_file", return_value=bots_config + ), mock.patch("zulip_botserver.server.lib.ExternalBotHandler", new=mock.Mock()), mock.patch( + "zulip_bots.finder.metadata.EntryPoint.load", + return_value=packaged_bot_module, + ), mock.patch( + "zulip_bots.finder.metadata.entry_points", + return_value=(packaged_bot_entrypoint,), + ): + server.main() + + mock_app.config.__setitem__.assert_any_call( + "BOTS_LIB_MODULES", {"packaged_bot": packaged_bot_module} + ) + if __name__ == "__main__": unittest.main() diff --git a/zulip_botserver/zulip_botserver/server.py b/zulip_botserver/zulip_botserver/server.py index 7dc9abfac..17d051180 100644 --- a/zulip_botserver/zulip_botserver/server.py +++ b/zulip_botserver/zulip_botserver/server.py @@ -1,8 +1,6 @@ #!/usr/bin/env python3 import configparser -import importlib.abc -import importlib.util import json import logging import os @@ -18,6 +16,7 @@ from zulip import Client from zulip_bots import lib +from zulip_bots.finder import import_module_from_source, import_module_from_zulip_bot_registry from zulip_botserver.input_parameters import parse_args @@ -112,41 +111,34 @@ def parse_config_file(config_file_path: str) -> configparser.ConfigParser: return parser -# TODO: Could we use the function from the bots library for this instead? -def load_module_from_file(file_path: str) -> ModuleType: - # Wrapper around importutil; see https://stackoverflow.com/a/67692/3909240. - spec = importlib.util.spec_from_file_location("custom_bot_module", file_path) - lib_module = importlib.util.module_from_spec(spec) - assert isinstance(spec.loader, importlib.abc.Loader) - spec.loader.exec_module(lib_module) - return lib_module - - -def load_lib_modules(available_bots: List[str]) -> Dict[str, Any]: +def load_lib_modules(available_bots: List[str]) -> Dict[str, ModuleType]: bots_lib_module = {} for bot in available_bots: try: if bot.endswith(".py") and os.path.isfile(bot): - lib_module = load_module_from_file(bot) + lib_module = import_module_from_source(bot, "custom_bot_module") else: module_name = "zulip_bots.bots.{bot}.{bot}".format(bot=bot) lib_module = import_module(module_name) bots_lib_module[bot] = lib_module except ImportError: - error_message = ( - 'Error: Bot "{}" doesn\'t exist. Please make sure ' - "you have set up the botserverrc file correctly.\n".format(bot) - ) - if bot == "api": - error_message += ( - "Did you forget to specify the bot you want to run with -b ?" + _, bots_lib_module[bot] = import_module_from_zulip_bot_registry(bot) + if bots_lib_module[bot] is None: + error_message = ( + f'Error: Bot "{bot}" doesn\'t exist. Please make sure ' + "you have set up the botserverrc file correctly.\n" ) - sys.exit(error_message) + if bot == "api": + error_message += ( + "Did you forget to specify the bot you want to run with -b ?" + ) + sys.exit(error_message) return bots_lib_module def load_bot_handlers( available_bots: List[str], + bot_lib_modules: Dict[str, ModuleType], bots_config: Dict[str, Dict[str, str]], third_party_bot_conf: Optional[configparser.ConfigParser] = None, ) -> Dict[str, lib.ExternalBotHandler]: @@ -157,7 +149,7 @@ def load_bot_handlers( api_key=bots_config[bot]["key"], site=bots_config[bot]["site"], ) - bot_dir = os.path.join(os.path.dirname(os.path.abspath(__file__)), "bots", bot) + bot_dir = os.path.join(os.path.dirname(os.path.abspath(bot_lib_modules[bot].__file__))) bot_handler = lib.ExternalBotHandler( client, bot_dir, bot_details={}, bot_config_parser=third_party_bot_conf ) @@ -253,7 +245,9 @@ def main() -> None: third_party_bot_conf = ( parse_config_file(options.bot_config_file) if options.bot_config_file is not None else None ) - bot_handlers = load_bot_handlers(available_bots, bots_config, third_party_bot_conf) + bot_handlers = load_bot_handlers( + available_bots, bots_lib_modules, bots_config, third_party_bot_conf + ) message_handlers = init_message_handlers(available_bots, bots_lib_modules, bot_handlers) app.config["BOTS_LIB_MODULES"] = bots_lib_modules app.config["BOT_HANDLERS"] = bot_handlers