Skip to content

Conversation

@tommasobertoni
Copy link
Contributor

@tommasobertoni tommasobertoni commented Aug 31, 2022

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 [ ])

  • 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.

@tommasobertoni
Copy link
Contributor Author

💡 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 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 seratch self-assigned this Aug 31, 2022
@seratch 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 seratch added this to the 3.18.2 milestone Aug 31, 2022
)


@pytest.mark.parametrize("initial_blocks", [
Copy link
Contributor

Choose a reason for hiding this comment

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

nice!

@seratch
Copy link
Contributor

seratch commented Aug 31, 2022

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
Copy link
Contributor

seratch commented Aug 31, 2022

@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
Copy link

codecov bot commented Aug 31, 2022

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

@seratch seratch merged commit 9c2ae69 into slackapi:main Sep 1, 2022
@ydshieh
Copy link

ydshieh commented Sep 5, 2022

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
Copy link
Contributor

seratch commented Sep 5, 2022

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
Copy link

ydshieh commented Sep 5, 2022

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

@ydshieh
Copy link

ydshieh commented Sep 5, 2022

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
Copy link
Contributor

seratch commented Sep 6, 2022

@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
Copy link
Contributor

seratch commented Sep 6, 2022

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

@ydshieh
Copy link

ydshieh commented Sep 6, 2022

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

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 Version: 3x web-client

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

3 participants