diff --git a/zulip_bots/zulip_bots/lib.py b/zulip_bots/zulip_bots/lib.py index de33510..fbb3f7c 100644 --- a/zulip_bots/zulip_bots/lib.py +++ b/zulip_bots/zulip_bots/lib.py @@ -88,8 +88,14 @@ class StateHandler(object): return key in self.state_ class ExternalBotHandler(object): - def __init__(self, client, root_dir, bot_details, bot_config_file): - # type: (Client, str, Dict[str, Any], str) -> None + def __init__( + self, + client: Client, + root_dir: str, + bot_details: Dict[str, Any], + bot_config_file: Optional[str]=None, + bot_config_parser: Optional[configparser.ConfigParser]=None, + ) -> None: # Only expose a subset of our Client's functionality try: user_profile = client.get_profile() @@ -114,6 +120,7 @@ class ExternalBotHandler(object): self._root_dir = root_dir self.bot_details = bot_details self.bot_config_file = bot_config_file + self._bot_config_parser = bot_config_parser self._storage = StateHandler(client) try: self.user_id = user_profile['user_id'] @@ -156,51 +163,51 @@ class ExternalBotHandler(object): else: self._rate_limit.show_error_and_exit() - def get_config_info(self, bot_name, optional=False): - # type: (str, Optional[bool]) -> Dict[str, Any] + def get_config_info(self, bot_name: str, optional: Optional[bool]=False) -> Dict[str, Any]: + if self._bot_config_parser is not None: + config_parser = self._bot_config_parser + else: + if self.bot_config_file is None: + if optional: + return dict() - if self.bot_config_file is None: - if optional: - return dict() + # Well written bots should catch this exception + # and provide nice error messages with instructions + # on setting up the configuration specfic to this bot. + # And then `run.py` should also catch exceptions on how + # to specify the file in the command line. + raise NoBotConfigException(bot_name) - # Well written bots should catch this exception - # and provide nice error messages with instructions - # on setting up the configuration specfic to this bot. - # And then `run.py` should also catch exceptions on how - # to specify the file in the command line. - raise NoBotConfigException(bot_name) + if bot_name not in self.bot_config_file: + print(''' + WARNING! - if bot_name not in self.bot_config_file: - print(''' - WARNING! + {} does not adhere to the + file naming convention, and it could be a + sign that you passed in the + wrong third-party configuration file. - {} does not adhere to the - file naming convention, and it could be a - sign that you passed in the - wrong third-party configuration file. + The suggested name is {}.conf - The suggested name is {}.conf + We will proceed anyway. + '''.format(self.bot_config_file, bot_name)) - We will proceed anyway. - '''.format(self.bot_config_file, bot_name)) + # We expect the caller to pass in None if the user does + # not specify a bot_config_file. If they pass in a bogus + # filename, we'll let an IOError happen here. Callers + # like `run.py` will do the command line parsing and checking + # for the existence of the file. + config_parser = configparser.ConfigParser() + with open(self.bot_config_file) as conf: + try: + config_parser.read(conf) + except configparser.Error as e: + display_config_file_errors(str(e), self.bot_config_file) + sys.exit(1) - # We expect the caller to pass in None if the user does - # not specify a bot_config_file. If they pass in a bogus - # filename, we'll let an IOError happen here. Callers - # like `run.py` will do the command line parsing and checking - # for the existence of the file. - config = configparser.ConfigParser() - with open(self.bot_config_file) as conf: - try: - config.readfp(conf) # type: ignore # readfp->read_file in python 3, so not in stubs - except configparser.Error as e: - display_config_file_errors(str(e), self.bot_config_file) - sys.exit(1) + return dict(config_parser.items(bot_name)) - return dict(config.items(bot_name)) - - def open(self, filepath): - # type: (str) -> IO[str] + def open(self, filepath: str) -> IO[str]: filepath = os.path.normpath(filepath) abs_filepath = os.path.join(self._root_dir, filepath) if abs_filepath.startswith(self._root_dir): diff --git a/zulip_botserver/tests/server_test_lib.py b/zulip_botserver/tests/server_test_lib.py index 5c20c62..19d82c9 100644 --- a/zulip_botserver/tests/server_test_lib.py +++ b/zulip_botserver/tests/server_test_lib.py @@ -16,7 +16,7 @@ class BotServerTestCase(TestCase): available_bots: Optional[List[str]]=None, bots_config: Optional[Dict[str, Dict[str, str]]]=None, bots_lib_module: Optional[Dict[str, Any]]=None, - bot_handlers: Optional[Union[Dict[str, Any], BadRequest]]=None, + bot_handlers: Optional[Dict[str, Any]]=None, payload_url: str="/bots/helloworld", message: Optional[Dict[str, Any]]=dict(message={'key': "test message"}), check_success: bool=False, @@ -26,9 +26,10 @@ class BotServerTestCase(TestCase): bots_lib_modules = server.load_lib_modules() server.app.config["BOTS_LIB_MODULES"] = bots_lib_modules if bot_handlers is None: - bot_handlers = server.load_bot_handlers(bots_config, bots_lib_modules) - if not isinstance(bot_handlers, BadRequest): - server.app.config["BOT_HANDLERS"] = bot_handlers + bot_handlers = server.load_bot_handlers(bots_config) + message_handlers = server.init_message_handlers(bots_lib_modules, bot_handlers) + server.app.config["BOT_HANDLERS"] = bot_handlers + server.app.config["MESSAGE_HANDLERS"] = message_handlers response = self.app.post(payload_url, data=json.dumps(message)) diff --git a/zulip_botserver/zulip_botserver/input_parameters.py b/zulip_botserver/zulip_botserver/input_parameters.py index 134cdd3..691a865 100644 --- a/zulip_botserver/zulip_botserver/input_parameters.py +++ b/zulip_botserver/zulip_botserver/input_parameters.py @@ -24,6 +24,12 @@ def parse_args() -> argparse.Namespace: required=True, help='Config file for the zulip bot server (flaskbotrc)' ) + parser.add_argument( + '--bot-config-file', + action='store', + default=None, + help='Config file for third-party bots' + ) parser.add_argument( '--bot-name', '-b', action='store', diff --git a/zulip_botserver/zulip_botserver/server.py b/zulip_botserver/zulip_botserver/server.py index 24bf845..0c07835 100644 --- a/zulip_botserver/zulip_botserver/server.py +++ b/zulip_botserver/zulip_botserver/server.py @@ -9,18 +9,14 @@ from typing import Any, Dict, Union, List, Optional from werkzeug.exceptions import BadRequest from zulip import Client -from zulip_bots.lib import ExternalBotHandler, StateHandler +from zulip_bots.lib import ExternalBotHandler from zulip_botserver.input_parameters import parse_args available_bots = [] # type: List[str] def read_config_file(config_file_path: str, bot_name: Optional[str]=None) -> Dict[str, Dict[str, str]]: - config_file_path = os.path.abspath(os.path.expanduser(config_file_path)) - if not os.path.isfile(config_file_path): - raise IOError("Could not read config file {}: File not found.".format(config_file_path)) - parser = configparser.ConfigParser() - parser.read(config_file_path) + parser = parse_config_file(config_file_path) bots_config = {} for section in parser.sections(): @@ -33,12 +29,20 @@ def read_config_file(config_file_path: str, bot_name: Optional[str]=None) -> Dic bots_config[bot_name] = section_info logging.warning("First bot name in the config list was changed to '{}'. " "Other bots will be ignored".format(bot_name)) - break - else: - bots_config[section] = section_info + return bots_config + bots_config[section] = section_info return bots_config +def parse_config_file(config_file_path: str) -> configparser.ConfigParser: + config_file_path = os.path.abspath(os.path.expanduser(config_file_path)) + if not os.path.isfile(config_file_path): + raise IOError("Could not read config file {}: File not found.".format(config_file_path)) + parser = configparser.ConfigParser() + parser.read(config_file_path) + return parser + + def load_lib_modules() -> Dict[str, Any]: bots_lib_module = {} for bot in available_bots: @@ -56,53 +60,60 @@ def load_lib_modules() -> Dict[str, Any]: def load_bot_handlers( bots_config: Dict[str, Dict[str, str]], - bots_lib_module: Dict[str, Any], -) -> Union[Dict[str, ExternalBotHandler], BadRequest]: + bot_config_file: Optional[str]=None +) -> Dict[str, ExternalBotHandler]: bot_handlers = {} + third_party_bot_conf = parse_config_file(bot_config_file) if bot_config_file is not None else None for bot in available_bots: client = Client(email=bots_config[bot]["email"], api_key=bots_config[bot]["key"], site=bots_config[bot]["site"]) - try: - bot_dir = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'bots', bot) - # TODO: Figure out how to pass in third party config info. - bot_handler = ExternalBotHandler( - client, - bot_dir, - bot_details={}, - bot_config_file=None - ) - bot_handlers[bot] = bot_handler + bot_dir = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'bots', bot) + bot_handler = ExternalBotHandler( + client, + bot_dir, + bot_details={}, + bot_config_parser=third_party_bot_conf + ) - lib_module = bots_lib_module[bot] - message_handler = lib_module.handler_class() - if hasattr(message_handler, 'validate_config'): - config_data = bot_handlers[bot].get_config_info(bot) - lib_module.handler_class.validate_config(config_data) - - if hasattr(message_handler, 'initialize'): - message_handler.initialize(bot_handler=bot_handler) - except SystemExit: - return BadRequest("Cannot fetch user profile for bot {}, make sure you have set up the flaskbotrc " - "file correctly.".format(bot)) + bot_handlers[bot] = bot_handler return bot_handlers +def init_message_handlers( + bots_lib_modules: Dict[str, Any], + bot_handlers: Dict[str, ExternalBotHandler], +) -> Dict[str, Any]: + message_handlers = {} + for bot in available_bots: + bot_lib_module = bots_lib_modules[bot] + bot_handler = bot_handlers[bot] + message_handler = bot_lib_module.handler_class() + if hasattr(message_handler, 'validate_config'): + config_data = bot_handler.get_config_info(bot) + bot_lib_module.handler_class.validate_config(config_data) + + if hasattr(message_handler, 'initialize'): + message_handler.initialize(bot_handler=bot_handler) + message_handlers[bot] = message_handler + return message_handlers + + app = Flask(__name__) @app.route('/bots/', methods=['POST']) def handle_bot(bot: str) -> Union[str, BadRequest]: lib_module = app.config.get("BOTS_LIB_MODULES", {}).get(bot) + bot_handler = app.config.get("BOT_HANDLERS", {}).get(bot) + message_handler = app.config.get("MESSAGE_HANDLERS", {}).get(bot) if lib_module is None: return BadRequest("Can't find the configuration or Bot Handler code for bot {}. " "Make sure that the `zulip_bots` package is installed, and " "that your flaskbotrc is set up correctly".format(bot)) - message_handler = lib_module.handler_class() event = request.get_json(force=True) - message_handler.handle_message(message=event["message"], - bot_handler=app.config["BOT_HANDLERS"].get(bot)) + message_handler.handle_message(message=event["message"], bot_handler=bot_handler) return json.dumps("") @@ -112,9 +123,11 @@ def main() -> None: global available_bots available_bots = list(bots_config.keys()) bots_lib_modules = load_lib_modules() - bot_handlers = load_bot_handlers(bots_config, bots_lib_modules) + bot_handlers = load_bot_handlers(bots_config, options.bot_config_file) + message_handlers = init_message_handlers(bots_lib_modules, bot_handlers) app.config["BOTS_LIB_MODULES"] = bots_lib_modules app.config["BOT_HANDLERS"] = bot_handlers + app.config["MESSAGE_HANDLERS"] = message_handlers app.run(host=options.hostname, port=int(options.port), debug=True) if __name__ == '__main__':