Add basic slack CLI support #995
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #995 +/- ##
==========================================
+ Coverage 91.76% 91.87% +0.11%
==========================================
Files 181 189 +8
Lines 6312 6450 +138
==========================================
+ Hits 5792 5926 +134
- Misses 520 524 +4 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
At glance, the code looks great to me!
While checking this PR, I discovered that there's no need for the slack_bolt.cli code to be included in the slack_bolt package. If my understanding is correct , I would recommend creating a new package for all CLI-related stuff. The name for the new one can be slack-cli-hooks or whatever (note that slack-cli is already taken by a community developer).
The main benefit of having a smaller dedicated package for the CLI integration would be a different release cycle. We can start with 0.1.0 with a clear alpha/beta declaration. This makes the active development of the CLI integration much easier and faster.
|
@seratch I agree that isolating this code in a package brings the advantage of owning its release cycle Should the |
|
@WilliamBergamin the example template listed in the testing instructions above doesn't include a Custom Function, right? |
No it does not include a custom function |
|
@WilliamBergamin Either way, developers need to install required packges (at least slack-bolt and its underlying dependecny slack-sdk). If our CLI mechanism has a limitation that developers cannot smoothly import multiple PyPI packages,
I am down to this idea. With this approach, the slack-cli-hooks can resolve slack-bolt and slack-sdk automatically. Developers do not need to know what packages are required to spin up their apps. |
| "get-manifest": f"python -m {get_manifest.__name__}", | ||
| "start": f"python -m {start.__name__}", |
There was a problem hiding this comment.
Are there any concerns around using the python runtime? For example, my macOS system doesn't have a python binary installed and instead uses python3.
There was a problem hiding this comment.
That is a great catch 💯 I very much glanced over this, let me look into this and try to find a broader solution
There was a problem hiding this comment.
Using python3 instead of python seem to be the safer option
The goal of this PR is to introduce basic support for slack CLI in Bolt for python. This is accomplished by implementing the
get-hooks,get-manifestandstartinterface. Support fordefaultandmessage-boundariescommunication protocol was also implemented in order to leverage the full capabilities of the slack CLITesting
slack create -t https://github.com/slack-samples/bolt-python-starter-template -b cli-supportpip install -r requirements.txtpip uninstall slack-boltscripts/build_pypi_package.shthis will generate a.whlin thedist/folderpip install global/path/to/bolt-python/dist/something.whlmanifest.json"outgoing_domains": []"settings"."function_runtime": "remote""settings"."org_deploy_enabled": trueslack manifestslack runFeedback
slack runfor async apps but had issues implementing scenario tests for them, any ideas for this can helpCategory
slack_bolt.Appand/or its core componentsslack_bolt.async_app.AsyncAppand/or its core componentsslack_bolt.adapter/docsRequirements
Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.
./scripts/install_all_and_run_tests.shafter making the changes.