Skip to content

Conversation

@seratch
Copy link
Contributor

@seratch seratch commented May 29, 2023

Summary

This pull request resolves #1377

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 tests M-T: Testing work only web-client Version: 3x labels May 29, 2023
@seratch seratch added this to the 3.22.0 milestone May 29, 2023
@seratch seratch self-assigned this May 29, 2023
)
except aiohttp.ContentTypeError:
logger.debug(f"No response data returned from the following API call: {api_url}.")
retry_response = RetryHttpResponse(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

a potential bug

if self.status_code == 200 and self.data and (isinstance(self.data, bytes) or self.data.get("ok", False)):
return self
msg = f"The request to the Slack API failed. (url: {self.api_url})"
msg = f"The request to the Slack API failed. (url: {self.api_url}, status: {self.status_code})"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

for better debuggability

await client.auth_test()

@async_test
async def test_retries(self):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved from tests/slack_sdk_async/web/test_web_client_http_retry.py to reduce confusion for future maintainers

request: HttpRequest,
response: Optional[HttpResponse],
error: Optional[Exception],
response: Optional[HttpResponse] = None,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

for consistency

@codecov
Copy link

codecov bot commented May 29, 2023

Codecov Report

Merging #1378 (5823925) into main (b65d3fc) will increase coverage by 0.01%.
The diff coverage is 40.00%.

@@            Coverage Diff             @@
##             main    #1378      +/-   ##
==========================================
+ Coverage   85.56%   85.57%   +0.01%     
==========================================
  Files         111      111              
  Lines       11582    11586       +4     
==========================================
+ Hits         9910     9915       +5     
+ Misses       1672     1671       -1     
Impacted Files Coverage Δ
slack_sdk/audit_logs/v1/async_client.py 88.42% <0.00%> (-0.74%) ⬇️
slack_sdk/http_retry/builtin_async_handlers.py 95.65% <ø> (+4.34%) ⬆️
slack_sdk/scim/v1/async_client.py 93.61% <0.00%> (-0.67%) ⬇️
slack_sdk/webhook/async_client.py 91.34% <0.00%> (-0.89%) ⬇️
slack_sdk/web/async_internal_utils.py 78.64% <100.00%> (+0.20%) ⬆️
slack_sdk/web/async_slack_response.py 88.73% <100.00%> (ø)

... and 1 file with indirect coverage changes

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

@seratch seratch merged commit 432ed1e into slackapi:main May 29, 2023
@seratch seratch deleted the issue-1377-tests branch May 29, 2023 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented tests M-T: Testing work only Version: 3x web-client

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add unit tests for #1367 changes

1 participant