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

Enhance SlackReader to fetch Channel IDs from Channel Names/Patterns #14429

Merged

Conversation

chetanchoudhary-calsoft
Copy link
Contributor

@chetanchoudhary-calsoft chetanchoudhary-calsoft commented Jun 27, 2024

Summary

This PR enhances the SlackReader class to support filtering Slack channels based on channel name patterns, including exact names and regular expressions. This update makes it easier to specify and retrieve conversations from channels by adding method to get channel IDs using channel names or regex pattern.

Changes

  • Feature: Added support for extracting channel ids from channel name patterns (exact names and regex) in SlackReader.

  • Enhancement: Added the get_channel_ids method to fetch channel ids from name patterns, these ids can be further used in load_data same as before.

  • Bug fixes

    • Error Handling: If not_in_channel error or any other error except ratelimited is returned by Slack, The code went into infinite loop of indefinite retries. Improved handling of the not_in_channel error. The reader now logs this error and breaks the loop, preventing indefinite retries.
    • Rate Limiting: Enhanced rate-limiting logic by adding a default retry interval, ensuring controlled retries when the retry-after header is missing.

Motivation

The existing implementation required channel IDs to retrieve conversations, which could be cumbersome for users who may not have direct access to these IDs. By allowing the use of channel name patterns, this update simplifies the process of specifying channels, especially when dealing with multiple or dynamically named channels.

Testing

  • Verified that channels are correctly filtered based on provided name patterns.
  • Ensured that both exact name matches and regex pattern matches are handled appropriately.
  • Tested the load_data method with various combinations of channel IDs and name patterns to confirm correct functionality.

Please review the changes and provide feedback. Thank you!

- Added support for channel name patterns (exact names and regex) in SlackReader.
- Updated load_data method to handle both channel IDs and name patterns, combining results.
- Added support for channel name patterns (exact names and regex) in SlackReader.
- get_channel_ids method to get channel ids from given channel names or regex patterns.
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Jun 27, 2024
@chetanchoudhary-calsoft chetanchoudhary-calsoft marked this pull request as draft June 27, 2024 10:38
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@chetanchoudhary-calsoft chetanchoudhary-calsoft marked this pull request as ready for review June 28, 2024 12:04
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Jun 28, 2024
- Handle `not_in_channel` error by logging and exiting the loop to prevent indefinite retries.
- Implement a default retry interval for rate limiting, ensuring controlled retries.
- Enhances overall error handling and robustness.
Copy link
Contributor

@fersilva16 fersilva16 left a comment

Choose a reason for hiding this comment

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

lgtm

@chetanchoudhary-calsoft
Copy link
Contributor Author

@logan-markewich , please help with the review on this PR.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jul 2, 2024
@logan-markewich logan-markewich merged commit a2e9e68 into run-llama:main Jul 2, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants