slack-machine icon indicating copy to clipboard operation
slack-machine copied to clipboard

Async support

Open ramnes opened this issue 1 year ago • 7 comments

Hey there!

Do you have any plan for supporting async code?

I've seen that you want to use the Events API (#469), so maybe bolt-python and its async backend could be used as a base?

ramnes avatar Jul 20 '22 09:07 ramnes

Hey there! How serendipitous that you ask about this! I just got back to Slack Machine yesterday after a hiatus of a couple of months. I started experimenting with async yesterday. My plan is not only to support async, but to make Slack Machine async-only.

My initial idea was to use Bolt, but I think it's overkill for the low-level access I need. Slack SDK for Python actually also supports SocketMode and it can work with the Events API just as well.

I also debated for a while if Bolt was making Slack Machine obsolete, but from what I've seen, Bolt doesn't really let you build Slack bots in a scalable way, using a plugin-based architecture (see this issue for the problems with Bolt). This is what Slack Machine does well. So if I can implement async support with SocketMode and the Event API, I think it could really help adoption of Slack Machine

Anyways, thanks for your interest! I hope to build a first async version this weekend and release it as an alpha

DonDebonair avatar Jul 21 '22 13:07 DonDebonair

Haha, glad to see that the universe is properly aligned today!

I definitely understand your feel that it's overkill for what you're currently building, but basing your work on top of it would let you profit from Slack engineering effort. It seems like they really want to make this the standard for developing bots, so you presumably would be able to use their newer client features easily in the future. And right now you wouldn't have to reimplement what they already implement, making the surface of Slack Machine smaller, which is great for you as a maintainer, and for contributors as well.

Let's digress a bit: I'm currently using Bolt to build a Slack bot but it's too low-level for my tastes. What I personally dream of (on top of the async support) is a rule matching system based on type hints, à la FastAPI. I quite liked what you did so far with named groups and I think that's a good base. But instead of having a regexp for matching, and then having to cast variables manually in the function, I would love to use a simpler notation in the decorator, type hints in the function signature, and then receive parameters that are correctly typed already.

That's worth another issue for sure, but my point is: this is the kind of feature that would really give value to Slack Machine, and that will most likely never exist in Bolt... not reimplementing Bolt a bit differently. Also, your current plugin-based architecture and Bolt don't feel mutually exclusive to me: the base class could hold the app instance at the class level. Plugins would then register their methods when instantiating. There are probably other ways too!

I think the tradeoff is all about this, actually: if you think that it would take you more time adapting your work on top of Bolt rather than implementing the newer Slack client features, then you should definitely do that, but don't forget to consider the future maintenance. From my experience, it's always harder in the long run to keep up-to-date a codebase that reimplements things than one that's based on top of other people work. :)

Anyway, sorry for the long answer, and whatever you end up doing, I'm eager to try the asynchronous machine!

PS – Without any pretension, if you need help / recommandations on how to switch to async, I've made the transition on several projects and have a few ones that support both sync and async, so I'd be happy to answer any question!

ramnes avatar Jul 22 '22 09:07 ramnes

Thanks for your elaborate answer! Very insightful!

With regards to the underlying base of Slack Machine, there are basically 2 options:

I don't think the latter will receive less engineering effort from the Slack team than the former, because Bolt is actually built on top of the Python Slack SDK. The Python Slack SDK is just more low-level than Bolt. And what Bolt adds on top of the Python Slack SDK, is mostly convenience in a similar way as Slack Machine does. E.g. decorators for writing handlers for certain events.

But Slack Machine augments that with a true plugin system, that should make it easy to distribute functionality separate from your bot instance and reuse it between bots.

When I look at the code examples between Bolt and the Python Slack SDK, for my purposes there are not so many differences when it comes to using Socket Mode (which is what I'm focussing on right now).

So unless I'm missing something, there is no convincing reason to use Bolt instead of the Python Slack SDK, given that the latter will be maintained because it's used in the former?

With regards to the rule matching: when you talk about having typed arguments in your function and casting input automatically, how far would that go? What I mean is: FastAPI lets you define nested models (using Pydantic), which makes sense for an API with JSON input. For a Slack bot it's much simpeler, because you're always dealing with unstructured text (Slack messages) so the only casting you could do, is to int, bool, str and maybe date/datetime? Or am I missing something?

And you'd still need regex I think, because the text is unstructured?

Can you given an example of what a listener function would look like and what input it would respond to?

DonDebonair avatar Jul 22 '22 10:07 DonDebonair

Btw, I have a Gitter room setup (see README) to discuss things in chat, but it's not really used right now. We could use it to have more real-time in-depth discussions at some point. I also contemplated moving to Slack for the community.

DonDebonair avatar Jul 22 '22 10:07 DonDebonair

I don't think the latter will receive less engineering effort from the Slack team than the former, because Bolt is actually built on top of the Python Slack SDK. The Python Slack SDK is just more low-level than Bolt.

So unless I'm missing something, there is no convincing reason to use Bolt instead of the Python Slack SDK, given that the latter will be maintained because it's used in the former?

You're completely right. I was under the impression than Bolt was implementing most things async and events-related, but digging a bit more I'm discovering now that I completely overlooked the SDK, and that it's far more complete than I thought it was. Sorry for the noise!

With regards to the rule matching: when you talk about having typed arguments in your function and casting input automatically, how far would that go? What I mean is: FastAPI lets you define nested models (using Pydantic), which makes sense for an API with JSON input. For a Slack bot it's much simpeler, because you're always dealing with unstructured text (Slack messages) so the only casting you could do, is to int, bool, str and maybe date/datetime? Or am I missing something?

I was referring to the path parameters. :)

You can cast any path parameter to an int, a float, a Decimal, an UUID, a BSON ObjectId... or anything that can be casted from a string.

An alternative is Flask / Starlette "convertors", that work with regular expressions underneath, but they don't rely on type hints and thus feel a bit less modern.

ramnes avatar Jul 22 '22 13:07 ramnes

So, I made quite some progress on the async story! You can find my work in this draft PR. It's still WIP, but the basics work now:

  • The core is completely async, based on SocketMode + events
  • The plugin system is now async, so all plugin methods should be async as well
  • The built-in plugins have been converted to async as well
  • I've refactored a lot of stuff, adding type hints wherever I could

TODO:

  • Currently, only the @respond_to and @listen_to decorators work in plugins. @process is next on my todo list and should be really easy.
  • The RBAC plugin doesn't work yet
  • The scheduler has been removed completely. I'm not sure if I'll bring it back. Removing the scheduler allowed me to get rid of the Singleton pattern that was plaguing the code. And I'm not sure if anyone was using it or not. If there is interest to bring it back, I can! But I'll have to rethink how I can persist scheduled tasks
  • The exact type-matching for parameters extracted from the message (which you mentioned above) is something I'd like to do, but I'll have to look into how that would work
  • Update documentation

The work on the branch can already be tested. I've added a script in the root called run_dev_async.py which runs the async version of Slack Machine. The code for sync + async currently live side by side, with the async code living in machine_v2. I want to finish this and start testing it rigorously. If it works, and some users (like you?) have tested it as well, I actually want to remove the sync code, because I don't plan on maintaining 2 versions. People can always fall back to older versions if they want to.

If you want to test it, you'll have to create your own Slack app. You can use the following App Manifest to get started quickly:

App Manifest
display_information:
  name: Slack Machine
features:
  bot_user:
    display_name: Slack Machine
    always_online: false
oauth_config:
  scopes:
    bot:
      - app_mentions:read
      - channels:history
      - channels:join
      - channels:read
      - chat:write
      - chat:write.public
      - emoji:read
      - groups:history
      - groups:read
      - groups:write
      - im:history
      - im:read
      - im:write
      - mpim:history
      - mpim:read
      - mpim:write
      - pins:read
      - pins:write
      - reactions:read
      - reactions:write
      - users:read
      - users:read.email
      - channels:manage
      - chat:write.customize
      - commands
      - dnd:read
      - files:read
      - files:write
      - links:read
      - links:write
      - metadata.message:read
      - usergroups:read
      - usergroups:write
      - users.profile:read
      - users:write
settings:
  event_subscriptions:
    bot_events:
      - app_mention
      - channel_archive
      - channel_created
      - channel_deleted
      - channel_id_changed
      - channel_left
      - channel_rename
      - channel_unarchive
      - group_archive
      - group_deleted
      - group_left
      - group_rename
      - group_unarchive
      - member_joined_channel
      - member_left_channel
      - message.channels
      - message.groups
      - message.im
      - message.mpim
      - reaction_added
      - reaction_removed
      - team_join
      - user_change
      - user_profile_changed
      - user_status_changed
  interactivity:
    is_enabled: true
  org_deploy_enabled: false
  socket_mode_enabled: true
  token_rotation_enabled: false

DonDebonair avatar Aug 01 '22 21:08 DonDebonair

Update @process now works as well!

Async code has been moved to the machine.asyncio package

DonDebonair avatar Aug 02 '22 20:08 DonDebonair

Update: RBAC now works! And all of the async code is type checked with MyPy

DonDebonair avatar Aug 12 '22 20:08 DonDebonair

This has been fixed now that #592 has been merged. Published as v0.26.0

DonDebonair avatar Aug 13 '22 11:08 DonDebonair