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

Update deploy with aws serverless guide #1066

Merged
merged 2 commits into from
Aug 18, 2021

Conversation

srajiang
Copy link
Contributor

@srajiang srajiang commented Aug 17, 2021

Summary

Tweaks AWS Serverless deployment guide to include instructions for disabling SocketMode (from Getting Started App sample). Now that SocketMode is default in the Getting Started guide, devs who want to set up their app as Serverless Lambda function need to turn off SocketMode in favor of HTTP.

Also fixes #1062 by adding a warning.

Requirements (place an x in each [ ])

@gitwave gitwave bot added the untriaged label Aug 17, 2021
@srajiang srajiang requested a review from mwbrooks August 17, 2021 23:27
@srajiang srajiang self-assigned this Aug 17, 2021
@srajiang srajiang added docs M-T: Documentation work only and removed untriaged labels Aug 17, 2021
@srajiang srajiang marked this pull request as ready for review August 17, 2021 23:28
@codecov
Copy link

codecov bot commented Aug 17, 2021

Codecov Report

Merging #1066 (ac047d4) into main (dc3e4a9) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1066   +/-   ##
=======================================
  Coverage   68.89%   68.89%           
=======================================
  Files          13       13           
  Lines        1212     1212           
  Branches      357      357           
=======================================
  Hits          835      835           
  Misses        304      304           
  Partials       73       73           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dc3e4a9...ac047d4. Read the comment docs.

@srajiang srajiang requested a review from misscoded August 17, 2021 23:31
@@ -108,15 +108,26 @@ Now that you have an app, let's prepare it for AWS Lambda and the Serverless Fra

**1. Prepare the app for AWS Lambda**

By default, Bolt listens for HTTP requests. In this section, we'll customize your Bolt app's [`receiver`](https://slack.dev/bolt-js/concepts#receiver) to respond to Lambda function events instead.
By default, our Bolt Getting Started app sample is configured to use SocketMode. Let's update the setup in `app.js` to have our app listen for HTTP requests instead.
Copy link
Member

Choose a reason for hiding this comment

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

Nice 👍


```javascript
const { App, AwsLambdaReceiver } = require('@slack/bolt');
```

Then update the [source code that initializes your Bolt app](https://github.com/slackapi/bolt-js-getting-started-app/blob/main/app.js#L3-L7) to create a custom receiver using AwsLambdaReceiver:
Then update the [source code that initializes your Bolt app](https://github.com/slackapi/bolt-js-getting-started-app/blob/main/app.js#L10-L14) to create a custom receiver using AwsLambdaReceiver:
Copy link
Member

Choose a reason for hiding this comment

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

This may not be a change that we want to have this time (as the existing one had the same room for improvement) but let me share a minor suggestion.

L10-14 in the main branch can be different in the future. If we point specific lines, we can use a specific revision for it (and we may want to update the revision if it becomes very old after a while). https://github.com/slackapi/bolt-js-getting-started-app/blob/4c29a21438b40f0cbca71ece0d39b356dfcf88d5/app.js#L10-L14

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah interesting. Linking the revision keeps the link from breaking due to direct changes. I like that! I've updated both links based on that feedback. 🙇

@seratch
Copy link
Member

seratch commented Aug 17, 2021

@srajiang Would it be possible to ask you to fix #1062 as well in this pull request?

@seratch seratch added this to the 3.6.0 milestone Aug 17, 2021
@seratch
Copy link
Member

seratch commented Aug 18, 2021

Thanks for updating this PR! Looks great to me. This pull request fixes #1062 as well

@seratch seratch merged commit 1b1231a into slackapi:main Aug 18, 2021
@srajiang srajiang deleted the socket-mode-serverless-instructs branch August 18, 2021 00:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs M-T: Documentation work only
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update AWS Lambda deployment guide to have notice about OAuth support
2 participants