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

Updating lazy lambda OAuth example #444

Merged
merged 2 commits into from
Aug 13, 2021
Merged

Updating lazy lambda OAuth example #444

merged 2 commits into from
Aug 13, 2021

Conversation

filmaj
Copy link
Contributor

@filmaj filmaj commented Aug 12, 2021

Updating the lazy lambda OAuth example with instructions

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.

@filmaj filmaj requested a review from seratch August 12, 2021 14:30
@seratch seratch added area:adapter area:examples issues related to example or sample code docs Improvements or additions to documentation labels Aug 12, 2021
@seratch seratch added this to the 1.8.0 milestone Aug 12, 2021

sys.path.insert(1, "vendor")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't look like this works. AWS Chalice seems to include vendor/ automatically but AWS Lambda not so much. I can't find more information on this, but I do remember before this did work! But, it has been probably 2 years or so since I've used Python in Lambda, so perhaps that behaviour has changed.

Copy link
Member

Choose a reason for hiding this comment

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

Good catch 👍

- `SLACK_INSTALLATION_S3_BUCKET_NAME`: The name of one of the S3 buckets you
created
- `SLACK_STATE_S3_BUCKET_NAME`: The name of the other S3 bucket you created
- `SLACK_LAMBDA_PATH`: ??? TODO
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@seratch what is this env var used for, do you know?

Copy link
Member

Choose a reason for hiding this comment

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

This is no longer needed. You can safely delete it!

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 the example 👍

- `SLACK_INSTALLATION_S3_BUCKET_NAME`: The name of one of the S3 buckets you
created
- `SLACK_STATE_S3_BUCKET_NAME`: The name of the other S3 bucket you created
- `SLACK_LAMBDA_PATH`: ??? TODO
Copy link
Member

Choose a reason for hiding this comment

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

This is no longer needed. You can safely delete it!


sys.path.insert(1, "vendor")
Copy link
Member

Choose a reason for hiding this comment

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

Good catch 👍

@codecov
Copy link

codecov bot commented Aug 12, 2021

Codecov Report

Merging #444 (f67ee2d) into main (df43450) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #444   +/-   ##
=======================================
  Coverage   91.34%   91.34%           
=======================================
  Files         167      167           
  Lines        5499     5499           
=======================================
  Hits         5023     5023           
  Misses        476      476           

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 df43450...f67ee2d. Read the comment docs.

@seratch seratch merged commit 008758e into main Aug 13, 2021
@seratch seratch deleted the lazy-lambda-oauth-example branch August 13, 2021 04:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:adapter area:examples issues related to example or sample code docs Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants