Skip to content

Fix #1143 Keep Retry-After header compatibility wit 2.x series#1148

Merged
seratch merged 1 commit intoslackapi:mainfrom
seratch:issue-1144-retry-after-compat
Dec 10, 2021
Merged

Fix #1143 Keep Retry-After header compatibility wit 2.x series#1148
seratch merged 1 commit intoslackapi:mainfrom
seratch:issue-1144-retry-after-compat

Conversation

@seratch
Copy link
Copy Markdown
Contributor

@seratch seratch commented Dec 9, 2021

Summary

This pull request fixes #1143, which is a regression bug since v3.12.0. For code consistency, I've updated all the clients but actually only WebClient had this issue.

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.

@seratch seratch added bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented web-client Version: 3x scim-client audit-logs-client labels Dec 9, 2021
@seratch seratch added this to the 3.13.0 milestone Dec 9, 2021
@seratch seratch self-assigned this Dec 9, 2021
if "ratelimited" in pattern:
self.send_response(429)
self.send_header("Retry-After", 1)
self.send_header("retry-after", 1)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changed the mock API server's behavior -- with this, the existing test assets detect the lack of the Retry-After header

@codecov
Copy link
Copy Markdown

codecov bot commented Dec 9, 2021

Codecov Report

Merging #1148 (a3bae59) into main (a58ab92) will increase coverage by 0.02%.
The diff coverage is 81.81%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1148      +/-   ##
==========================================
+ Coverage   86.62%   86.65%   +0.02%     
==========================================
  Files         110      110              
  Lines       10755    10760       +5     
==========================================
+ Hits         9317     9324       +7     
+ Misses       1438     1436       -2     
Impacted Files Coverage Δ
slack_sdk/web/base_client.py 89.10% <75.00%> (+0.43%) ⬆️
slack_sdk/web/legacy_base_client.py 87.75% <75.00%> (+0.46%) ⬆️
slack_sdk/audit_logs/v1/client.py 90.69% <100.00%> (+0.07%) ⬆️
slack_sdk/scim/v1/client.py 93.33% <100.00%> (+0.04%) ⬆️
slack_sdk/webhook/client.py 93.91% <100.00%> (+0.05%) ⬆️

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 a58ab92...a3bae59. Read the comment docs.

@seratch seratch force-pushed the issue-1144-retry-after-compat branch from 5a768d2 to a3bae59 Compare December 9, 2021 09:55
@seratch seratch changed the title Fix #1144 Keep Retry-After header compatibility wit 2.x series Fix #1143 Keep Retry-After header compatibility wit 2.x series Dec 9, 2021
@seratch seratch merged commit b5823f8 into slackapi:main Dec 10, 2021
@seratch seratch deleted the issue-1144-retry-after-compat branch December 10, 2021 00:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Capitalized Retry-After header missing since 3.12.0

1 participant