Skip to content
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

Slack Trigger - Incorrect assumption about API response leads to erros #1057

Closed
AleksanderGondek opened this issue Feb 8, 2021 · 0 comments · Fixed by #1058
Closed

Slack Trigger - Incorrect assumption about API response leads to erros #1057

AleksanderGondek opened this issue Feb 8, 2021 · 0 comments · Fixed by #1058
Labels
bug Something isn't working

Comments

@AleksanderGondek
Copy link
Contributor

Describe the bug
Slack Trigger, iterates all slack channels returned from the API (via pagination) to find appropriate channel ID - that implementation makes incorrect assumption - it expects the count of returned items to always be equal to pagination limit, unless last page is returned.

Such assumption clearly goes again the Slack API design - see link, which states that returned chunk of data may be smaller than pagination limit, even if that is not the last page or results.

With large enough and dynamic enough Slack Workspace, aforementioned implementation leads to 'failed to get channelID of %s' exceptions, even if channel with appropriate name exists - the pagination simply stops, before it can reach proper item.

To Reproduce
Steps to reproduce the behavior:

  1. Have Slack API return list of channels which does not neatly have 200 items per page
  2. Choose channel to sent message to, which is at the end of the pagination
  3. Attempt to run Slack Trigger
  4. See error 'failed to get channelID of %s'

Expected behavior
I would expect the Slack Trigger to work as long as the channel with given name exists and is returned by the API pagination, irrespectively of pages size.

Environment (please complete the following information):

  • Kubernetes: v1.19.6
  • Argo: v2.12.7
  • Argo Events: v1.2.2

Additional context
Proposed fix: Simply paginate until returned cursor is empty.


Message from the maintainers:

If you wish to see this enhancement implemented please add a 👍 reaction to this issue! We often sort issues this way to know what to prioritize.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant