bots: Indicate source of bot (from source/module/registry) upon startup.
Amend tests to include new parameter.
This commit is contained in:
		
							parent
							
								
									4bc0c607c1
								
							
						
					
					
						commit
						66434d07cf
					
				
					 5 changed files with 47 additions and 9 deletions
				
			
		|  | @ -32,7 +32,7 @@ class DuplicateRegisteredBotName(Exception): | ||||||
|     pass |     pass | ||||||
| 
 | 
 | ||||||
| 
 | 
 | ||||||
| def import_module_from_zulip_bot_registry(name: str) -> Optional[ModuleType]: | 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 |     # Prior to Python 3.10, calling importlib.metadata.entry_points returns a | ||||||
|     # SelectableGroups object when no parameters is given. Currently we use |     # SelectableGroups object when no parameters is given. Currently we use | ||||||
|     # the importlib_metadata library for compatibility, but we need to migrate |     # the importlib_metadata library for compatibility, but we need to migrate | ||||||
|  | @ -42,12 +42,38 @@ def import_module_from_zulip_bot_registry(name: str) -> Optional[ModuleType]: | ||||||
|     matching_bots = [bot for bot in registered_bots if bot.name == name] |     matching_bots = [bot for bot in registered_bots if bot.name == name] | ||||||
| 
 | 
 | ||||||
|     if len(matching_bots) == 1:  # Unique matching entrypoint |     if len(matching_bots) == 1:  # Unique matching entrypoint | ||||||
|         return matching_bots[0].load() |         """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: |     if len(matching_bots) > 1: | ||||||
|         raise DuplicateRegisteredBotName(name) |         raise DuplicateRegisteredBotName(name) | ||||||
| 
 | 
 | ||||||
|     return None  # no matches in registry |     return "", None  # no matches in registry | ||||||
| 
 | 
 | ||||||
| 
 | 
 | ||||||
| def resolve_bot_path(name: str) -> Optional[Tuple[Path, str]]: | def resolve_bot_path(name: str) -> Optional[Tuple[Path, str]]: | ||||||
|  |  | ||||||
|  | @ -443,6 +443,7 @@ def run_message_handler_for_bot( | ||||||
|     config_file: str, |     config_file: str, | ||||||
|     bot_config_file: str, |     bot_config_file: str, | ||||||
|     bot_name: str, |     bot_name: str, | ||||||
|  |     bot_source: str, | ||||||
| ) -> Any: | ) -> Any: | ||||||
|     """ |     """ | ||||||
|     lib_module is of type Any, since it can contain any bot's |     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) |     message_handler = prepare_message_handler(bot_name, restricted_client, lib_module) | ||||||
| 
 | 
 | ||||||
|     if not quiet: |     if not quiet: | ||||||
|         print("Running {} Bot:".format(bot_details["name"])) |         print("Running {} Bot (from {}):".format(bot_details["name"], bot_source)) | ||||||
|         if bot_details["description"] != "": |         if bot_details["description"] != "": | ||||||
|             print("\n\t{}".format(bot_details["description"])) |             print("\n\t{}".format(bot_details["description"])) | ||||||
|         if hasattr(message_handler, "usage"): |         if hasattr(message_handler, "usage"): | ||||||
|  |  | ||||||
|  | @ -118,7 +118,7 @@ def main() -> None: | ||||||
| 
 | 
 | ||||||
|     if args.registry: |     if args.registry: | ||||||
|         try: |         try: | ||||||
|             lib_module = finder.import_module_from_zulip_bot_registry(args.bot) |             bot_source, lib_module = finder.import_module_from_zulip_bot_registry(args.bot) | ||||||
|         except finder.DuplicateRegisteredBotName as error: |         except finder.DuplicateRegisteredBotName as error: | ||||||
|             print( |             print( | ||||||
|                 f'ERROR: Found duplicate entries for "{error}" in zulip bots registry.\n' |                 f'ERROR: Found duplicate entries for "{error}" in zulip bots registry.\n' | ||||||
|  | @ -151,10 +151,12 @@ def main() -> None: | ||||||
|                 ) |                 ) | ||||||
|                 print(dep_err_msg.format(bot_name=bot_name, deps_list=deps_list)) |                 print(dep_err_msg.format(bot_name=bot_name, deps_list=deps_list)) | ||||||
|                 sys.exit(1) |                 sys.exit(1) | ||||||
|  |             bot_source = "source" | ||||||
|         else: |         else: | ||||||
|             lib_module = finder.import_module_by_name(args.bot) |             lib_module = finder.import_module_by_name(args.bot) | ||||||
|             if lib_module: |             if lib_module: | ||||||
|                 bot_name = lib_module.__name__ |                 bot_name = lib_module.__name__ | ||||||
|  |                 bot_source = "named module" | ||||||
|                 if args.provision: |                 if args.provision: | ||||||
|                     print("ERROR: Could not load bot's module for '{}'. Exiting now.") |                     print("ERROR: Could not load bot's module for '{}'. Exiting now.") | ||||||
|                     sys.exit(1) |                     sys.exit(1) | ||||||
|  | @ -179,6 +181,7 @@ def main() -> None: | ||||||
|             bot_config_file=args.bot_config_file, |             bot_config_file=args.bot_config_file, | ||||||
|             quiet=args.quiet, |             quiet=args.quiet, | ||||||
|             bot_name=bot_name, |             bot_name=bot_name, | ||||||
|  |             bot_source=bot_source, | ||||||
|         ) |         ) | ||||||
|     except NoBotConfigException: |     except NoBotConfigException: | ||||||
|         print( |         print( | ||||||
|  |  | ||||||
|  | @ -197,6 +197,7 @@ class LibTest(TestCase): | ||||||
|                 config_file=None, |                 config_file=None, | ||||||
|                 bot_config_file=None, |                 bot_config_file=None, | ||||||
|                 bot_name="testbot", |                 bot_name="testbot", | ||||||
|  |                 bot_source="bot code location", | ||||||
|             ) |             ) | ||||||
| 
 | 
 | ||||||
|     def test_upload_file(self): |     def test_upload_file(self): | ||||||
|  |  | ||||||
|  | @ -2,13 +2,13 @@ | ||||||
| import os | import os | ||||||
| import sys | import sys | ||||||
| import unittest | import unittest | ||||||
| from importlib.metadata import EntryPoint |  | ||||||
| from pathlib import Path | from pathlib import Path | ||||||
| from typing import Optional | from typing import Optional | ||||||
| from unittest import TestCase, mock | from unittest import TestCase, mock | ||||||
| from unittest.mock import patch | from unittest.mock import MagicMock, patch | ||||||
| 
 | 
 | ||||||
| import zulip_bots.run | import zulip_bots.run | ||||||
|  | from zulip_bots.finder import metadata | ||||||
| from zulip_bots.lib import extract_query_without_mention | from zulip_bots.lib import extract_query_without_mention | ||||||
| 
 | 
 | ||||||
| 
 | 
 | ||||||
|  | @ -16,7 +16,10 @@ class TestDefaultArguments(TestCase): | ||||||
| 
 | 
 | ||||||
|     our_dir = os.path.dirname(__file__) |     our_dir = os.path.dirname(__file__) | ||||||
|     path_to_bot = os.path.abspath(os.path.join(our_dir, "../bots/giphy/giphy.py")) |     path_to_bot = os.path.abspath(os.path.join(our_dir, "../bots/giphy/giphy.py")) | ||||||
|     packaged_bot_entrypoint = EntryPoint("packaged_bot", "module_name", "zulip_bots.registry") |     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("sys.argv", ["zulip-run-bot", "giphy", "--config-file", "/foo/bar/baz.conf"]) | ||||||
|     @patch("zulip_bots.run.run_message_handler_for_bot") |     @patch("zulip_bots.run.run_message_handler_for_bot") | ||||||
|  | @ -31,6 +34,7 @@ class TestDefaultArguments(TestCase): | ||||||
|             config_file="/foo/bar/baz.conf", |             config_file="/foo/bar/baz.conf", | ||||||
|             bot_config_file=None, |             bot_config_file=None, | ||||||
|             lib_module=mock.ANY, |             lib_module=mock.ANY, | ||||||
|  |             bot_source="source", | ||||||
|             quiet=False, |             quiet=False, | ||||||
|         ) |         ) | ||||||
| 
 | 
 | ||||||
|  | @ -47,6 +51,7 @@ class TestDefaultArguments(TestCase): | ||||||
|             config_file="/foo/bar/baz.conf", |             config_file="/foo/bar/baz.conf", | ||||||
|             bot_config_file=None, |             bot_config_file=None, | ||||||
|             lib_module=mock.ANY, |             lib_module=mock.ANY, | ||||||
|  |             bot_source="source", | ||||||
|             quiet=False, |             quiet=False, | ||||||
|         ) |         ) | ||||||
| 
 | 
 | ||||||
|  | @ -58,7 +63,8 @@ class TestDefaultArguments(TestCase): | ||||||
|         self, mock_run_message_handler_for_bot: mock.Mock |         self, mock_run_message_handler_for_bot: mock.Mock | ||||||
|     ) -> None: |     ) -> None: | ||||||
|         with patch("zulip_bots.run.exit_gracefully_if_zulip_config_is_missing"), patch( |         with patch("zulip_bots.run.exit_gracefully_if_zulip_config_is_missing"), patch( | ||||||
|             "zulip_bots.finder.metadata.EntryPoint.load" |             "zulip_bots.finder.metadata.EntryPoint.load", | ||||||
|  |             return_value=self.packaged_bot_module, | ||||||
|         ), patch( |         ), patch( | ||||||
|             "zulip_bots.finder.metadata.entry_points", |             "zulip_bots.finder.metadata.entry_points", | ||||||
|             return_value=(self.packaged_bot_entrypoint,), |             return_value=(self.packaged_bot_entrypoint,), | ||||||
|  | @ -70,6 +76,7 @@ class TestDefaultArguments(TestCase): | ||||||
|             config_file="/foo/bar/baz.conf", |             config_file="/foo/bar/baz.conf", | ||||||
|             bot_config_file=None, |             bot_config_file=None, | ||||||
|             lib_module=mock.ANY, |             lib_module=mock.ANY, | ||||||
|  |             bot_source="packaged_bot: 1.0.0", | ||||||
|             quiet=False, |             quiet=False, | ||||||
|         ) |         ) | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue
	
	 PIG208
						PIG208