Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

botserver: Allow importing custom bot modules. #523

Closed
wants to merge 2 commits into from

Conversation

aero31aero
Copy link
Member

We can now specify path to a bot's python file as the ini section
header in the botserver's config file. For example:

[~/Documents/helloworld.py]
email[email protected]
key=XXXX
site=https://b.com
token=XXXX

aero31aero added a commit to aero31aero/zulip that referenced this pull request Aug 28, 2019
@timabbott
Copy link
Member

Looks reasonable; is it easy to add a test for it?

# 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)
spec.loader.exec_module(lib_module) # type: ignore # mypy thinks the `spec` is None in Optional[Loader].
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can use assert spec is not None for this. Though what happens if the file path is invalid? We might need code for that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is called from inside a try-except block, and we print:

except ImportError:
            error_message = ("Error: Bot \"{}\" doesn't exist. Please make sure "
                             "you have set up the botserverrc file correctly.\n".format(bot))

Copy link
Member Author

@aero31aero aero31aero Aug 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(A slightly related issue in mypy: python/mypy#2420)

I'm working on tests for this, that should add additional checking for our purposes.

timabbott pushed a commit to aero31aero/zulip that referenced this pull request Aug 28, 2019
aero31aero added a commit to aero31aero/zulip that referenced this pull request Aug 29, 2019
We can now specify path to a bot's python file as the ini section
header in the botserver's config file. For example:

[~/Documents/helloworld.py]
[email protected]
key=XXXX
site=https://b.com
token=XXXX
@aero31aero aero31aero force-pushed the 0-botserver-custom-module branch from ee3089c to 602c442 Compare August 30, 2019 01:08
@zulipbot zulipbot added size: M and removed size: S labels Aug 30, 2019
@aero31aero aero31aero force-pushed the 0-botserver-custom-module branch from 602c442 to ba9d901 Compare August 30, 2019 01:15
@aero31aero
Copy link
Member Author

Added tests, this is ready for review along with zulip/zulip#13090.

@aero31aero aero31aero force-pushed the 0-botserver-custom-module branch from ba9d901 to 9a0bb61 Compare August 30, 2019 01:33
@timabbott
Copy link
Member

Squashed and merged, thanks @aero31aero!

@timabbott timabbott closed this Sep 3, 2019
timabbott pushed a commit to zulip/zulip that referenced this pull request Sep 3, 2019
matheuscmelo pushed a commit to matheuscmelo/zulip that referenced this pull request Sep 28, 2019
YashRE42 pushed a commit to YashRE42/zulip that referenced this pull request Dec 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants