Skip to content

Improve #1084 by removing globally mutable lists#1092

Merged
seratch merged 1 commit intoslackapi:mainfrom
seratch:pr-1084-mutable-lists
Aug 13, 2021
Merged

Improve #1084 by removing globally mutable lists#1092
seratch merged 1 commit intoslackapi:mainfrom
seratch:pr-1084-mutable-lists

Conversation

@seratch
Copy link
Copy Markdown
Contributor

@seratch seratch commented Aug 13, 2021

Summary

This pull request removes all the globally mutable list objects from the pull request #1084 changes. If we have those, mutating the list of retry handlers can cause side effects to other API client instances in the same app.

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 ./docs.sh?)
  • /docs-src-v2 (Documents, have you run ./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 Aug 13, 2021
@seratch seratch added this to the 3.9.0 milestone Aug 13, 2021
@codecov
Copy link
Copy Markdown

codecov bot commented Aug 13, 2021

Codecov Report

Merging #1092 (271265b) into main (561a42f) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1092      +/-   ##
==========================================
+ Coverage   86.17%   86.19%   +0.02%     
==========================================
  Files         110      110              
  Lines        9928     9932       +4     
==========================================
+ Hits         8555     8561       +6     
+ Misses       1373     1371       -2     
Impacted Files Coverage Δ
slack_sdk/audit_logs/v1/async_client.py 89.16% <ø> (ø)
slack_sdk/audit_logs/v1/client.py 91.20% <ø> (ø)
slack_sdk/scim/v1/async_client.py 94.20% <ø> (ø)
slack_sdk/scim/v1/client.py 93.75% <ø> (ø)
slack_sdk/web/async_base_client.py 98.33% <ø> (ø)
slack_sdk/web/base_client.py 89.55% <ø> (ø)
slack_sdk/webhook/async_client.py 92.23% <ø> (ø)
slack_sdk/webhook/client.py 94.59% <ø> (ø)
slack_sdk/http_retry/__init__.py 100.00% <100.00%> (ø)
slack_sdk/http_retry/builtin_async_handlers.py 95.00% <100.00%> (+0.12%) ⬆️
... and 2 more

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 561a42f...271265b. Read the comment docs.

@seratch seratch merged commit 2b30100 into slackapi:main Aug 13, 2021
@seratch seratch deleted the pr-1084-mutable-lists branch August 13, 2021 09:13
seratch added a commit to seratch/python-slack-sdk that referenced this pull request Aug 13, 2021
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.

1 participant