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

Fix #545 Enable to use lazy listeners even when having any custom context data #546

Merged
merged 4 commits into from
Dec 14, 2021

Conversation

seratch
Copy link
Member

@seratch seratch commented Dec 13, 2021

This pull request fixes #545 by enhancing the internals of lazy listener execution mechanism.

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 added bug Something isn't working area:async area:sync labels Dec 13, 2021
@seratch seratch added this to the 1.11.0 milestone Dec 13, 2021
@seratch seratch self-assigned this Dec 13, 2021
@codecov
Copy link

codecov bot commented Dec 13, 2021

Codecov Report

Merging #546 (eaf56a5) into main (e8556c1) will increase coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #546      +/-   ##
==========================================
+ Coverage   91.29%   91.33%   +0.04%     
==========================================
  Files         169      169              
  Lines        5606     5635      +29     
==========================================
+ Hits         5118     5147      +29     
  Misses        488      488              
Impacted Files Coverage Δ
slack_bolt/context/async_context.py 100.00% <100.00%> (ø)
slack_bolt/context/base_context.py 100.00% <100.00%> (ø)
slack_bolt/context/context.py 100.00% <100.00%> (ø)
slack_bolt/listener/asyncio_runner.py 85.96% <100.00%> (ø)
slack_bolt/listener/thread_runner.py 93.96% <100.00%> (ø)
slack_bolt/request/async_request.py 94.59% <100.00%> (+0.30%) ⬆️
slack_bolt/request/request.py 97.22% <100.00%> (+0.16%) ⬆️

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 e8556c1...eaf56a5. Read the comment docs.

@eddyg
Copy link
Contributor

eddyg commented Dec 13, 2021

Although “copiable” seems to be an accepted variant, a Google Ngram search shows “copyable” is more commonly used.

I bring this up because if somebody greps the code for “copy”, naming the methods to_copyable would match.

Just a small observation (functionally the code looks OK to me 🙂)

@seratch
Copy link
Member Author

seratch commented Dec 13, 2021

@eddyg Thanks! I'm happy to rename it 👍

@seratch
Copy link
Member Author

seratch commented Dec 13, 2021

Renamed the methods

@seratch seratch merged commit dc09c4c into slackapi:main Dec 14, 2021
@seratch seratch deleted the issue-545-lazy-listener-context branch December 14, 2021 03:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:async area:sync bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fail to make a deep copy of request with custom props for lazy listeners
2 participants