Fix #1258 Tuple value for blocks argument does not work for Web API calls by tommasobertoni · Pull Request #1259 · slackapi/python-slack-sdk

tommasobertoni · 2022-08-31T15:41:01Z

Summary

Fix for Tuple value for blocks argument does not work for Web API calls

blocks: Sequence[Block] = (
    SectionBlock(text="foo"),
    SectionBlock(text="bar"),
)

# fixes calls with non-list blocks and attachments
client.chat_postMessage(channel="#channel", blocks=blocks)

Resolves #1258

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

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.

tommasobertoni · 2022-08-31T15:42:09Z

💡 Additionally, the SDK could accept an Iterable instead of Sequence, therefore allowing generators like:

messages_to_send: list[str] = ...

client.chat_postMessage(
    channel="#channel",
    blocks=(SectionBlock(text=some_text) for some_text in messages_to_send)
)

seratch · 2022-08-31T21:57:33Z

tests/slack_sdk/web/test_internal_utils.py

        )
+
+
+@pytest.mark.parametrize("initial_blocks", [


seratch · 2022-08-31T21:58:47Z

Additionally, the SDK could accept an Iterable instead of Sequence, therefore allowing generators

Yeah, this can be a great addition for some specific use cases but not this time!

seratch · 2022-08-31T21:59:30Z

@tommasobertoni Thanks a lot for sending this PR! The changes already look great to me. Once the CI builds pass, we will merge the change into the main branch.

codecov · 2022-08-31T22:04:14Z

Codecov Report

Merging #1259 (beaf082) into main (d2fd7ac) will increase coverage by 0.07%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1259      +/-   ##
==========================================
+ Coverage   86.56%   86.64%   +0.07%     
==========================================
  Files         111      111              
  Lines       11026    11026              
==========================================
+ Hits         9545     9553       +8     
+ Misses       1481     1473       -8

Impacted Files	Coverage Δ
slack_sdk/web/internal_utils.py	`94.44% <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

ydshieh · 2022-09-05T12:05:44Z

Hi @tommasobertoni and @seratch , since the release of 3.18.2 (hence this PR being merged I believe), our CIs failed to send CI reports to our slack channels. The failing payload is provided below. The same payload was able to be sent to slack channels if I changed to slack_sdk==3.18.1.

Is this an intended breaking change?

The payload working with `3.18.1` but not `3.18.2`

{"blocks": [{"type": "header", "text": {"type": "plain_text", "text": "\ud83e\udd17 Results of the Past CI - pytorch-1.11 tests."}}, {"type": "section", "text": {"type": "plain_text", "text": "There were 68 failures, out of 28636 tests.\nThe suite ran in 2h34m15s.", "emoji": true}, "accessory": {"type": "button", "text": {"type": "plain_text", "text": "Check Action results", "emoji": true}, "url": "https://github.com/huggingface/transformers/actions/runs/2970011128"}}, {"type": "section", "text": {"type": "mrkdwn", "text": "The following modeling categories had failures:\n```\nSingle |  Multi | Category\n    14 |     15 | PyTorch\n     3 |      3 | TensorFlow\n     1 |      1 | Tokenizers\n     1 |      0 | Trainer\n     1 |      1 | Auto\n    14 |     14 | Unclassified\n```\n"}}, {"type": "section", "text": {"type": "mrkdwn", "text": "These following model modules had failures:\n```\nSingle PT |  Multi PT | Single TF |  Multi TF |     Other | Category\n        0 |         0 |         0 |         0 |         1 | trainer\n        0 |         0 |         0 |         0 |         2 | models_layoutlmv2\n        0 |         0 |         0 |         0 |        30 | models_wav2vec2_with_lm\n        0 |         0 |         3 |         3 |         0 | models_opt\n        1 |         1 |         0 |         0 |         0 | models_wav2vec2\n        3 |         3 |         0 |         0 |         0 | models_bloom\n       10 |        11 |         0 |         0 |         0 | models_owlvit\n```\n"}}, {"type": "section", "text": {"type": "mrkdwn", "text": "The following non-model modules had failures:\n```\nSingle |  Multi | Category\n     1 |      0 | trainer\n```\n"}}]}

Error

Traceback (most recent call last):
  File "utils/notification_service.py", line 901, in <module>
    message.post()
  File "utils/notification_service.py", line 445, in post
    self.thread_ts = client.chat_postMessage(
  File "/home/runner/.local/lib/python3.8/site-packages/slack_sdk/web/client.py", line 2066, in chat_postMessage
    return self.api_call("chat.postMessage", json=kwargs)
  File "/home/runner/.local/lib/python3.8/site-packages/slack_sdk/web/base_client.py", line 156, in api_call
    return self._sync_send(api_url=api_url, req_args=req_args)
  File "/home/runner/.local/lib/python3.8/site-packages/slack_sdk/web/base_client.py", line 187, in _sync_send
    return self._urllib_api_call(
  File "/home/runner/.local/lib/python3.8/site-packages/slack_sdk/web/base_client.py", line [30](https://github.com/huggingface/transformers/runs/8138562211?check_suite_focus=true#step:5:31)9, in _urllib_api_call
    return SlackResponse(
  File "/home/runner/.local/lib/python3.8/site-packages/slack_sdk/web/slack_response.py", line 189, in validate
    raise e.SlackApiError(message=msg, response=self)
slack_sdk.errors.SlackApiError: The request to the Slack API failed. (url: https://www.slack.com/api/chat.postMessage)
The server responded with: {'ok': False, 'error': 'invalid_blocks_format'}

seratch · 2022-09-05T12:26:20Z

Hi @ydshieh, thanks for reporting the issue. Definitely any existing code should work without any changes. I will look into this but, with given information, I am still unsure about how to reproduce your situation. If you can share the code snippet to reproduce the issue, that'd be greatly helpful for us.

ydshieh · 2022-09-05T12:33:14Z

@seratch Thanks, I will try to give a code snippet.

ydshieh · 2022-09-05T13:13:52Z

Hi @seratch

The script to reproduce the issue under slack_sdk==3.18.2
https://github.com/huggingface/transformers/blob/0b1d41e499dcfa6825f6e415c25d0d454e5ad05e/utils/notification_service.py

If helpful, this workflow file triggers the above script
https://github.com/huggingface/transformers/blob/reproduce-slack-api-issue/.github/workflows/self-scheduled.yml

Here are the job run results:

Let me know if you need more information, thanks

seratch · 2022-09-06T04:45:55Z

@ydshieh Thanks for sharing the details! It was very helpful for us to understand your use case. Using str value for blocks/attachments was not supported by design but we will support it in future versions. See also: #1261

seratch · 2022-09-06T06:53:43Z

@ydshieh Your issue should be resolved in the latest version 3.18.3. Thanks again for sharing this!

ydshieh · 2022-09-06T08:55:35Z

Thanks a lot, @seratch! Confirmed it works again.

tommasobertoni added 2 commits August 31, 2022 11:45

fix: parse sequences

aed26cf

test: can parse sequence of blocks and attachments

beaf082

seratch changed the title ~~fix: issue 1258~~ Fix #1258 Tuple value for blocks argument does not work for Web API calls Aug 31, 2022

seratch self-assigned this Aug 31, 2022

seratch added bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented web-client Version: 3x labels Aug 31, 2022

seratch added this to the 3.18.2 milestone Aug 31, 2022

seratch approved these changes Aug 31, 2022

View reviewed changes

tests/slack_sdk/web/test_internal_utils.py

)

@pytest.mark.parametrize("initial_blocks", [

Copy link
Copy Markdown

Contributor

seratch Aug 31, 2022

Choose a reason for hiding this comment

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

nice!

seratch merged commit 9c2ae69 into slackapi:main Sep 1, 2022

seratch mentioned this pull request Sep 6, 2022

blocks/attachments as str for chat.* API calls should be clearly supported #1261

Closed

9 tasks

seratch mentioned this pull request Sep 6, 2022

Fix #1261 blocks/attachments as str for chat.* API calls should be clearly supported #1262

Merged

16 tasks

ydshieh mentioned this pull request Sep 6, 2022

unpin slack_sdk version huggingface/transformers#18901

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #1258 Tuple value for blocks argument does not work for Web API calls#1259

Fix #1258 Tuple value for blocks argument does not work for Web API calls#1259
seratch merged 2 commits intoslackapi:mainfrom
tommasobertoni:fix/issue_1258

tommasobertoni commented Aug 31, 2022 •

edited

Loading

Uh oh!

tommasobertoni commented Aug 31, 2022

Uh oh!

seratch Aug 31, 2022

Uh oh!

seratch commented Aug 31, 2022

Uh oh!

seratch commented Aug 31, 2022

Uh oh!

codecov bot commented Aug 31, 2022

Uh oh!

ydshieh commented Sep 5, 2022 •

edited

Loading

Uh oh!

seratch commented Sep 5, 2022

Uh oh!

ydshieh commented Sep 5, 2022

Uh oh!

ydshieh commented Sep 5, 2022 •

edited

Loading

Uh oh!

seratch commented Sep 6, 2022

Uh oh!

seratch commented Sep 6, 2022

Uh oh!

ydshieh commented Sep 6, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

tommasobertoni commented Aug 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

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

Requirements (place an x in each [ ])

Uh oh!

tommasobertoni commented Aug 31, 2022

Uh oh!

seratch Aug 31, 2022

Choose a reason for hiding this comment

Uh oh!

seratch commented Aug 31, 2022

Uh oh!

seratch commented Aug 31, 2022

Uh oh!

codecov bot commented Aug 31, 2022

Codecov Report

Uh oh!

ydshieh commented Sep 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The payload working with 3.18.1 but not 3.18.2

Error

Uh oh!

seratch commented Sep 5, 2022

Uh oh!

ydshieh commented Sep 5, 2022

Uh oh!

ydshieh commented Sep 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

seratch commented Sep 6, 2022

Uh oh!

seratch commented Sep 6, 2022

Uh oh!

ydshieh commented Sep 6, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tommasobertoni commented Aug 31, 2022 •

edited

Loading

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

Requirements (place an `x` in each `[ ]`)

ydshieh commented Sep 5, 2022 •

edited

Loading

The payload working with `3.18.1` but not `3.18.2`

ydshieh commented Sep 5, 2022 •

edited

Loading