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.

Sign up for GitHub

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

Jump to bottom

Change AWS API Gateway to Lambda Function URL #661

Merged
merged 2 commits into from
May 29, 2022
Merged

Change AWS API Gateway to Lambda Function URL #661

merged 2 commits into from
May 29, 2022

Conversation

globart
Copy link
Contributor

@globart globart commented May 27, 2022

Update "AWS Lambda Bolt Python Examples" page to include Lambda URL

Category (place an x in each of the [ ])

  • slack_bolt.App and/or its core components
  • slack_bolt.async_app.AsyncApp and/or its core components
  • Adapters in slack_bolt.adapter
  • Document pages under /docs
  • Others

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.

  • I've read and understood the Contributing Guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.
  • I've run ./scripts/install_all_and_run_tests.sh after making the changes.

@seratch seratch self-assigned this May 27, 2022
@seratch seratch added docs Improvements or additions to documentation area:adapter area:examples issues related to example or sample code labels May 27, 2022
@seratch seratch added this to the 1.x milestone May 27, 2022
@CLAassistant
Copy link

CLAassistant commented May 27, 2022 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@seratch
Copy link
Member

seratch commented May 27, 2022

@Globart1337 Thank you very much for submitting this PR quickly! Since I live in a bit different timezone (+09:00), I am unable to check your pull request now. My teammates may be able to check this PR today.

One thing that I would like to ask you is to check the above comment by CLA bot. We are unable to merge your contributions without your CLA signature. Please check the CLA content and consider signing it 🙇

@globart
Copy link
Contributor Author

globart commented May 27, 2022

@Globart1337 Thank you very much for submitting this PR quickly! Since I live in a bit different timezone (+09:00), I am unable to check your pull request now. My teammates may be able to check this PR today.

One thing that I would like to ask you is to check the above comment by CLA bot. We are unable to merge your contributions without your CLA signature. Please check the CLA content and consider signing it bow

signed ✔️

@codecov
Copy link

codecov bot commented May 27, 2022 •
edited
Loading

Codecov Report

Merging #661 (c4fc5c0) into main (15033fe) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #661   +/-   ##
=======================================
  Coverage   92.05%   92.05%           
=======================================
  Files         172      172           
  Lines        5864     5864           
=======================================
  Hits         5398     5398           
  Misses        466      466

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 15033fe...c4fc5c0. Read the comment docs.

@filmaj filmaj self-requested a review May 27, 2022 20:00
filmaj
filmaj suggested changes May 27, 2022
View reviewed changes
Copy link
Contributor

@filmaj filmaj left a comment

Choose a reason for hiding this comment

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

Hey I tested this out and this is working great! Huge improvement, and simpler setup. I like this a lot 👍

The only request I would have is that at the top of this document, between lines 6 and 11, there are still a few references to API Gateway. Can we update those, please?

@globart
Copy link
Contributor Author

globart commented May 27, 2022

Hey I tested this out and this is working great! Huge improvement, and simpler setup. I like this a lot 👍

The only request I would have is that at the top of this document, between lines 6 and 11, there are still a few references to API Gateway. Can we update those, please?

Oh, didn't see them, will make an update

@globart globart requested a review from filmaj May 27, 2022 20:56
@seratch seratch modified the milestones: 1.x, 1.14.1 May 27, 2022
@globart
Copy link
Contributor Author

globart commented May 28, 2022

@filmaj please review

@globart
Copy link
Contributor Author

globart commented May 29, 2022

@seratch please review the change to PR or atleast ping @filmaj

seratch
seratch approved these changes May 29, 2022
View reviewed changes
Copy link
Member

@seratch seratch left a comment

Choose a reason for hiding this comment

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

Thanks for updating this PR. Looks good to me 👍

@seratch seratch merged commit cfc9daa into slackapi:main May 29, 2022
@seratch seratch mentioned this pull request May 29, 2022
Closed
4 tasks
@filmaj
Copy link
Contributor

filmaj commented May 30, 2022

Thanks a lot for this @Globart1337 ! Sorry I didn't respond earlier: it was my weekend so I try to stay away from the computer 😄

@globart
Copy link
Contributor Author

globart commented May 31, 2022

Thanks a lot for this @Globart1337 ! Sorry I didn't respond earlier: it was my weekend so I try to stay away from the computer 😄

Oh, okay then, no worries 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seratch seratch seratch approved these changes

@filmaj filmaj Awaiting requested review from filmaj

Assignees

@seratch seratch

Labels
area:adapter area:examples issues related to example or sample code docs Improvements or additions to documentation
Projects
None yet
Milestone
1.14.1
Development

Successfully merging this pull request may close these issues.
4 participants
@globart @CLAassistant @seratch @filmaj