-
Notifications
You must be signed in to change notification settings - Fork 850
Fix #1100 by improving the get/__getitem__ method behavior when response body is empty #1103
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -74,6 +74,10 @@ def __init__( | |
|
|
||
| def __str__(self): | ||
| """Return the Response data if object is converted to a string.""" | ||
| if isinstance(self.data, bytes): | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While SlackResponse has this validation, the same thing has been missing in |
||
| raise ValueError( | ||
| "As the response.data is binary data, this operation is unsupported" | ||
| ) | ||
| return f"{self.data}" | ||
|
|
||
| def __getitem__(self, key): | ||
|
|
@@ -87,6 +91,14 @@ def __getitem__(self, key): | |
| Returns: | ||
| The value from data or None. | ||
| """ | ||
| if isinstance(self.data, bytes): | ||
| raise ValueError( | ||
| "As the response.data is binary data, this operation is unsupported" | ||
| ) | ||
| if self.data is None: | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is added for resolving the issue #1100 |
||
| raise ValueError( | ||
| "As the response.data is empty, this operation is unsupported" | ||
| ) | ||
| return self.data.get(key, None) | ||
|
|
||
| def __aiter__(self): | ||
|
|
@@ -156,6 +168,12 @@ def get(self, key, default=None): | |
| Returns: | ||
| The value from data or the specified default. | ||
| """ | ||
| if isinstance(self.data, bytes): | ||
| raise ValueError( | ||
| "As the response.data is binary data, this operation is unsupported" | ||
| ) | ||
| if self.data is None: | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. get method should return |
||
| return None | ||
| return self.data.get(key, default) | ||
|
|
||
| def validate(self): | ||
|
|
@@ -168,13 +186,6 @@ def validate(self): | |
| Raises: | ||
| SlackApiError: The request to the Slack API failed. | ||
| """ | ||
| if self._logger.level <= logging.DEBUG: | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should have been removed in #1094 |
||
| self._logger.debug( | ||
| "Received the following response - " | ||
| f"status: {self.status_code}, " | ||
| f"headers: {dict(self.headers)}, " | ||
| f"body: {self.data}" | ||
| ) | ||
| if self.status_code == 200 and self.data and self.data.get("ok", False): | ||
| return self | ||
| msg = "The request to the Slack API failed." | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| import unittest | ||
|
|
||
| from slack.web.async_slack_response import AsyncSlackResponse | ||
| from slack_sdk.web.async_client import AsyncWebClient | ||
|
|
||
|
|
||
| class TestAsyncSlackResponse(unittest.TestCase): | ||
| def setUp(self): | ||
| pass | ||
|
|
||
| def tearDown(self): | ||
| pass | ||
|
|
||
| # https://github.com/slackapi/python-slack-sdk/issues/1100 | ||
| def test_issue_1100(self): | ||
| response = AsyncSlackResponse( | ||
| client=AsyncWebClient(token="xoxb-dummy"), | ||
| http_verb="POST", | ||
| api_url="http://localhost:3000/api.test", | ||
| req_args={}, | ||
| data=None, | ||
| headers={}, | ||
| status_code=200, | ||
| ) | ||
| with self.assertRaises(ValueError): | ||
| response["foo"] | ||
|
|
||
| foo = response.get("foo") | ||
| self.assertIsNone(foo) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For backward compatibility, we have two
AsynSlackResponseunderslackandslack_sdkpackages. We have to apply the same changes to the two this time.