zulip / python-zulip-api

Python library for the Zulip API.
https://zulip.com/api/
Apache License 2.0
350 stars 356 forks source link

Make a base class or protocol for ExternalBotHandler that zulip/zulip’s EmbeddedBotHandler can subclass #639

Closed andersk closed 3 years ago

andersk commented 3 years ago

zulip/zulip currently needs an unchecked cast to convert a custom EmbeddedBotHandler class into an ExternalBotHandler. Since EmbeddedBotHandler is not a subclass of ExternalBotHandler (because it works by invoking internal_send_message rather than making API calls), this is unsafe, and it breaks in practice when we try to upgrade python-zulip-api to 0.7.1 (zulip/zulip#16511).

To fix this, we need a common base class or Mypy protocol, say BotHandler, that can be subclassed by both ExternalBotHandler and EmbeddedBotHandler. The zulip_bots functions must be relaxed to accept BotHandler instead of requiring ExternalBotHandler.

If the StubBotHandler class used in tests could subclass BotHandler too, I think this would also let us correct various bot_handler: Any types to bot_handler: BotHandler.

LoopThrough-i-j commented 3 years ago

@andersk can I work on this?

LoopThrough-i-j commented 3 years ago

@zulipbot claim

zulipbot commented 3 years ago

Hello @LoopThrough-i-j!

Thanks for your interest in Zulip! You have attempted to claim an issue without the labels "help wanted", "good first issue". Since you're a new contributor, you can only claim and submit pull requests for issues with the help wanted or good first issue labels.

If this is your first time here, we recommend reading our guide for new contributors before getting started.

ahmedabuamra commented 3 years ago

@zulipbot claim

zulipbot commented 3 years ago

Hello @ahmedabuamra, it looks like you've currently claimed 1 issue in this repository. We encourage new contributors to focus their efforts on at most 1 issue at a time, so please complete your work on your other claimed issues before trying to claim this issue again.

We look forward to your valuable contributions!

ahmedabuamra commented 3 years ago

@andersk Can I work on this issue as the in progress one is in review?

LoopThrough-i-j commented 3 years ago

@zulipbot claim

zulipbot commented 3 years ago

Hello @LoopThrough-i-j!

Thanks for your interest in Zulip! You have attempted to claim an issue without the labels "help wanted", "good first issue". Since you're a new contributor, you can only claim and submit pull requests for issues with the help wanted or good first issue labels.

If this is your first time here, we recommend reading our guide for new contributors before getting started.

LoopThrough-i-j commented 3 years ago

@zulipbot claim

zulipbot commented 3 years ago

Welcome to Zulip, @LoopThrough-i-j! We just sent you an invite to collaborate on this repository at https://github.com/zulip/python-zulip-api/invitations. Please accept this invite in order to claim this issue and begin a fun, rewarding experience contributing to Zulip!

Here's some tips to get you off to a good start:

As you work on this issue, you'll also want to refer to the Zulip code contribution guide, as well as the rest of the developer documentation on that site.

See you on the other side (that is, the pull request side)!