Skip to content

fix retry_handlers type in AsyncBaseClient#1264

Merged
seratch merged 1 commit intoslackapi:mainfrom
ronyb29:patch-1
Sep 15, 2022
Merged

fix retry_handlers type in AsyncBaseClient#1264
seratch merged 1 commit intoslackapi:mainfrom
ronyb29:patch-1

Conversation

@ronyb29
Copy link
Copy Markdown
Contributor

@ronyb29 ronyb29 commented Sep 15, 2022

Summary

Fixing the types for retry_handlers in the async client, according to docs and the code itself it should have been AsyncRetryHandler from the beggning

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

  • slack_sdk.web.WebClient (sync/async) (Web API client)
  • slack_sdk.webhook.WebhookClient (sync/async) (Incoming Webhook, response_url sender)
  • slack_sdk.socket_mode (Socket Mode client)
  • slack_sdk.signature (Request Signature Verifier)
  • slack_sdk.oauth (OAuth Flow Utilities)
  • slack_sdk.models (UI component builders)
  • slack_sdk.scim (SCIM API client)
  • slack_sdk.audit_logs (Audit Logs API client)
  • slack_sdk.rtm_v2 (RTM client)
  • /docs-src (Documents, have you run ./scripts/docs.sh?)
  • /docs-src-v2 (Documents, have you run ./scripts/docs-v2.sh?)
  • /tutorial (PythOnBoardingBot tutorial)
  • tests/integration_tests (Automated tests for this library)

Requirements (place an x in each [ ])

  • 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 python3 -m venv .venv && source .venv/bin/activate && ./scripts/run_validation.sh after making the changes.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Sep 15, 2022

CLA assistant check
All committers have signed the CLA.

@seratch seratch self-assigned this Sep 15, 2022
@seratch seratch added bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented web-client Version: 3x area:async labels Sep 15, 2022
@seratch seratch added this to the 3.18.4 milestone Sep 15, 2022
@seratch
Copy link
Copy Markdown
Contributor

seratch commented Sep 15, 2022

Hi @ronyb29, thanks a lot for taking the time to send this PR! The changes look great to me but it seems that this is your first contribution to this project. Would you mind signing our CLA (see the above bot message for details)? Without it, we are unable to merge any contributions. It'd be appreciated if you could understand this.

@ronyb29
Copy link
Copy Markdown
Contributor Author

ronyb29 commented Sep 15, 2022

hey @seratch,
Thanks for pointing it out, just signed the CLA.

Copy link
Copy Markdown
Contributor

@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.

Thank you!

@codecov
Copy link
Copy Markdown

codecov bot commented Sep 15, 2022

Codecov Report

Merging #1264 (795ca77) into main (c782d71) will increase coverage by 0.07%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1264      +/-   ##
==========================================
+ Coverage   86.57%   86.64%   +0.07%     
==========================================
  Files         111      111              
  Lines       11026    11026              
==========================================
+ Hits         9546     9554       +8     
+ Misses       1480     1472       -8     
Impacted Files Coverage Δ
slack_sdk/web/async_base_client.py 98.33% <100.00%> (ø)
slack_sdk/socket_mode/builtin/client.py 90.24% <0.00%> (+0.60%) ⬆️
slack_sdk/socket_mode/builtin/connection.py 67.03% <0.00%> (+2.59%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@seratch seratch merged commit e239b16 into slackapi:main Sep 15, 2022
@ronyb29 ronyb29 deleted the patch-1 branch September 24, 2022 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:async bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented Version: 3x web-client

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants