Add a command line option to run.py for bot config files.
Before this change, we were looking for config files in default locations in source control, which is not a good place to look for them. Now `run.py` and friends have a command line argument where users can specify the config files. Note that the change to server.py is only a partial fix to make it so that bots that don't use third party config files won't crash. That program needs an overhaul, anyway.
This commit is contained in:
parent
57c90ddca0
commit
536ba1843a
|
@ -19,6 +19,9 @@ from types import ModuleType
|
|||
|
||||
from zulip import Client, ZulipError
|
||||
|
||||
class NoBotConfigException(Exception):
|
||||
pass
|
||||
|
||||
def exit_gracefully(signum, frame):
|
||||
# type: (int, Optional[Any]) -> None
|
||||
sys.exit(0)
|
||||
|
@ -83,8 +86,8 @@ class StateHandler(object):
|
|||
return key in self.state_
|
||||
|
||||
class ExternalBotHandler(object):
|
||||
def __init__(self, client, root_dir, bot_details):
|
||||
# type: (Client, str, Dict[str, Any]) -> None
|
||||
def __init__(self, client, root_dir, bot_details, bot_config_file):
|
||||
# type: (Client, str, Dict[str, Any], str) -> None
|
||||
# Only expose a subset of our Client's functionality
|
||||
try:
|
||||
user_profile = client.get_profile()
|
||||
|
@ -108,6 +111,7 @@ class ExternalBotHandler(object):
|
|||
self._client = client
|
||||
self._root_dir = root_dir
|
||||
self.bot_details = bot_details
|
||||
self.bot_config_file = bot_config_file
|
||||
self._storage = StateHandler(client)
|
||||
try:
|
||||
self.user_id = user_profile['user_id']
|
||||
|
@ -155,15 +159,45 @@ class ExternalBotHandler(object):
|
|||
|
||||
def get_config_info(self, bot_name, optional=False):
|
||||
# type: (str, Optional[bool]) -> Dict[str, Any]
|
||||
conf_file_path = os.path.realpath(os.path.join(self._root_dir, bot_name + '.conf'))
|
||||
config = configparser.ConfigParser()
|
||||
try:
|
||||
with open(conf_file_path) as conf:
|
||||
config.readfp(conf) # type: ignore # readfp->read_file in python 3, so not in stubs
|
||||
except IOError:
|
||||
|
||||
if self.bot_config_file is None:
|
||||
if optional:
|
||||
return dict()
|
||||
raise
|
||||
|
||||
# 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!
|
||||
|
||||
{} 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
|
||||
|
||||
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 = 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.items(bot_name))
|
||||
|
||||
def open(self, filepath):
|
||||
|
@ -206,8 +240,8 @@ def display_config_file_errors(error_msg, config_file):
|
|||
print('\nERROR: {} seems to be broken:\n\n{}'.format(config_file, file_contents))
|
||||
print('\nMore details here:\n\n{}\n'.format(error_msg))
|
||||
|
||||
def run_message_handler_for_bot(lib_module, quiet, config_file, bot_name):
|
||||
# type: (Any, bool, str, str) -> Any
|
||||
def run_message_handler_for_bot(lib_module, quiet, config_file, bot_config_file, bot_name):
|
||||
# type: (Any, bool, str, str, str) -> Any
|
||||
#
|
||||
# lib_module is of type Any, since it can contain any bot's
|
||||
# handler class. Eventually, we want bot's handler classes to
|
||||
|
@ -231,7 +265,7 @@ def run_message_handler_for_bot(lib_module, quiet, config_file, bot_name):
|
|||
sys.exit(1)
|
||||
|
||||
bot_dir = os.path.dirname(lib_module.__file__)
|
||||
restricted_client = ExternalBotHandler(client, bot_dir, bot_details)
|
||||
restricted_client = ExternalBotHandler(client, bot_dir, bot_details, bot_config_file)
|
||||
|
||||
message_handler = lib_module.handler_class()
|
||||
if hasattr(message_handler, 'initialize'):
|
||||
|
|
|
@ -11,7 +11,11 @@ from importlib import import_module
|
|||
from os.path import basename, splitext
|
||||
from typing import Any, Optional, Text
|
||||
|
||||
from zulip_bots.lib import run_message_handler_for_bot
|
||||
from zulip_bots.lib import (
|
||||
run_message_handler_for_bot,
|
||||
NoBotConfigException,
|
||||
)
|
||||
|
||||
from zulip_bots.provision import provision_bot
|
||||
|
||||
current_dir = os.path.dirname(os.path.abspath(__file__))
|
||||
|
@ -63,6 +67,10 @@ def parse_args():
|
|||
required=True,
|
||||
help='zulip configuration file (e.g. ~/Downloads/zuliprc)')
|
||||
|
||||
parser.add_argument('--bot-config-file', '-b',
|
||||
action='store',
|
||||
help='third party configuration file (e.g. ~/giphy.conf')
|
||||
|
||||
parser.add_argument('--force',
|
||||
action='store_true',
|
||||
help='try running the bot even if dependencies install fails')
|
||||
|
@ -75,7 +83,7 @@ def parse_args():
|
|||
return args
|
||||
|
||||
|
||||
def exit_gracefully_if_config_file_does_not_exist(config_file):
|
||||
def exit_gracefully_if_zulip_config_file_does_not_exist(config_file):
|
||||
# type: (str) -> None
|
||||
if not os.path.exists(config_file):
|
||||
print('''
|
||||
|
@ -87,6 +95,21 @@ def exit_gracefully_if_config_file_does_not_exist(config_file):
|
|||
''' % (config_file,))
|
||||
sys.exit(1)
|
||||
|
||||
def exit_gracefully_if_bot_config_file_does_not_exist(bot_config_file):
|
||||
# type: (str) -> None
|
||||
if bot_config_file is None:
|
||||
# This is a common case, just so succeed quietly. (Some
|
||||
# bots don't have third party configuration.)
|
||||
return
|
||||
|
||||
if not os.path.exists(bot_config_file):
|
||||
print('''
|
||||
ERROR: %s does not exist.
|
||||
|
||||
You probably just specified the wrong file location here.
|
||||
''' % (bot_config_file,))
|
||||
sys.exit(1)
|
||||
|
||||
def main():
|
||||
# type: () -> None
|
||||
args = parse_args()
|
||||
|
@ -103,14 +126,28 @@ def main():
|
|||
if not args.quiet:
|
||||
logging.basicConfig(stream=sys.stdout, level=logging.INFO)
|
||||
|
||||
exit_gracefully_if_config_file_does_not_exist(args.config_file)
|
||||
# It's a bit unfortunate that we have two config files, but the
|
||||
# alternative would be way worse for people running multiple bots
|
||||
# or testing against multiple Zulip servers.
|
||||
exit_gracefully_if_zulip_config_file_does_not_exist(args.config_file)
|
||||
exit_gracefully_if_bot_config_file_does_not_exist(args.bot_config_file)
|
||||
|
||||
run_message_handler_for_bot(
|
||||
lib_module=lib_module,
|
||||
config_file=args.config_file,
|
||||
quiet=args.quiet,
|
||||
bot_name=bot_name
|
||||
)
|
||||
try:
|
||||
run_message_handler_for_bot(
|
||||
lib_module=lib_module,
|
||||
config_file=args.config_file,
|
||||
bot_config_file=args.bot_config_file,
|
||||
quiet=args.quiet,
|
||||
bot_name=bot_name
|
||||
)
|
||||
except NoBotConfigException:
|
||||
print('''
|
||||
ERROR: Your bot requires you to specify a third party
|
||||
config file with the --bot-config-file option.
|
||||
|
||||
Exiting now.
|
||||
''')
|
||||
sys.exit(1)
|
||||
|
||||
if __name__ == '__main__':
|
||||
main()
|
||||
|
|
|
@ -44,7 +44,7 @@ class BotTestCaseBase(TestCase):
|
|||
# Mocking ExternalBotHandler
|
||||
self.patcher = patch('zulip_bots.lib.ExternalBotHandler', autospec=True)
|
||||
self.MockClass = self.patcher.start()
|
||||
self.mock_bot_handler = self.MockClass(None, None, None)
|
||||
self.mock_bot_handler = self.MockClass(None, None, None, None)
|
||||
self.mock_client = MagicMock()
|
||||
self.mock_client.get_storage.return_value = {'result': 'success', 'state': {}}
|
||||
self.mock_client.update_storage.return_value = {'result': 'success'}
|
||||
|
|
|
@ -29,11 +29,12 @@ class TestDefaultArguments(TestCase):
|
|||
@patch('zulip_bots.run.run_message_handler_for_bot')
|
||||
def test_argument_parsing_with_bot_name(self, mock_run_message_handler_for_bot):
|
||||
# type: (mock.Mock) -> None
|
||||
with patch('zulip_bots.run.exit_gracefully_if_config_file_does_not_exist'):
|
||||
with patch('zulip_bots.run.exit_gracefully_if_zulip_config_file_does_not_exist'):
|
||||
zulip_bots.run.main()
|
||||
|
||||
mock_run_message_handler_for_bot.assert_called_with(bot_name='giphy',
|
||||
config_file='/foo/bar/baz.conf',
|
||||
bot_config_file=None,
|
||||
lib_module=mock.ANY,
|
||||
quiet=False)
|
||||
|
||||
|
@ -41,12 +42,13 @@ class TestDefaultArguments(TestCase):
|
|||
@patch('zulip_bots.run.run_message_handler_for_bot')
|
||||
def test_argument_parsing_with_bot_path(self, mock_run_message_handler_for_bot):
|
||||
# type: (mock.Mock) -> None
|
||||
with patch('zulip_bots.run.exit_gracefully_if_config_file_does_not_exist'):
|
||||
with patch('zulip_bots.run.exit_gracefully_if_zulip_config_file_does_not_exist'):
|
||||
zulip_bots.run.main()
|
||||
|
||||
mock_run_message_handler_for_bot.assert_called_with(
|
||||
bot_name='giphy',
|
||||
config_file='/foo/bar/baz.conf',
|
||||
bot_config_file=None,
|
||||
lib_module=mock.ANY,
|
||||
quiet=False)
|
||||
|
||||
|
|
|
@ -54,7 +54,13 @@ def load_bot_handlers():
|
|||
try:
|
||||
bot_dir = os.path.join(os.path.dirname(os.path.abspath(__file__)),
|
||||
'bots', bot)
|
||||
bot_handlers[bot] = ExternalBotHandler(client, bot_dir, bot_details={})
|
||||
# TODO: Figure out how to pass in third party config info.
|
||||
bot_handlers[bot] = ExternalBotHandler(
|
||||
client,
|
||||
bot_dir,
|
||||
bot_details={},
|
||||
bot_config_file=None
|
||||
)
|
||||
except SystemExit:
|
||||
return BadRequest("Cannot fetch user profile for bot {}, make sure you have set up the flaskbotrc "
|
||||
"file correctly.".format(bot))
|
||||
|
|
Loading…
Reference in a new issue