-
Notifications
You must be signed in to change notification settings - Fork 248
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
Add Google Cloud Functions adapter (ref #646) #649
Add Google Cloud Functions adapter (ref #646) #649
Conversation
Codecov Report
@@ Coverage Diff @@
## main #649 +/- ##
==========================================
- Coverage 92.07% 92.05% -0.03%
==========================================
Files 170 172 +2
Lines 5833 5864 +31
==========================================
+ Hits 5371 5398 +27
- Misses 462 466 +4
Continue to review full report at Codecov.
|
Ah, yeah I should add some tests. Will do after finishing other tasks. |
|
||
def handle(self, req: Request) -> Response: | ||
if req.method == "GET": | ||
if self.app.oauth_flow is not None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can remove this extra if
Just change
if req.method == "GET":
if self.app.oauth_flow is not None:
to
if req.method == "GET" and self.app.oauth_flow is not None
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stasfilin Thanks for the suggestion!
Updated the code (thanks for the suggestion, @stasfilin!) and added unit tests ✅ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code LGTM but I did not create a test app using this as salesforce does not allow me to create Google Cloud projects 😑
def endpoint(): | ||
return SlackRequestHandler(app).handle(request) | ||
|
||
with flask_app.test_client() as client: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love this pattern.
|
||
|
||
# Step1: Create a new Slack App: https://api.slack.com/apps | ||
# Bot Token Scopes: chat:write, commands, app_mentions:read |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI the .env.yaml.oauth-sample has a different list than this; it includes channels.history
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I've updated the part.
This pull request adds a new adapter for Google Cloud Functions users. You can use Flask adapter for it as long as you don't have the OAuth flow in your app. However, as we see at #646, implementing the OAuth flow is far different and it would be confusing for developers. This pull request adds a new adapter that is quite similar to the AWS Lambda adapter.
One thing developers should be aware of would be the lazy listeners are not yet available for Google Cloud Functions, unlike AWS Lambda. Thus, I've added Noop runner, which just raises an exception when a developer tries to use lazy listeners in Cloud Functions app. Also, this is an improvement from the Flask adapter way.
Category (place an
x
in each of the[ ]
)slack_bolt.App
and/or its core componentsslack_bolt.async_app.AsyncApp
and/or its core componentsslack_bolt.adapter
/docs
Requirements (place an
x
in each[ ]
)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.sh
after making the changes.