-
Notifications
You must be signed in to change notification settings - Fork 259
Fix python version and slack bug #447
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
Conversation
|
""" WalkthroughThe changes update the Python version constraint in the project configuration, modify Slack file upload logic to use Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Runner
participant SlackAPI
User->>CLI: Run with --slackoutput <channel>
CLI->>Runner: Process results
Runner->>SlackAPI: Upload file via files_upload_v2 (no channel)
SlackAPI-->>Runner: Return file permalink
Runner->>SlackAPI: Post message with file permalink to channel via chat_postMessage
SlackAPI-->>Runner: Confirm message posted
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
robusta_krr/core/runner.py (1)
14-14: Remove unused import.The
SlackApiErrorimport is not used in the current implementation. Consider removing it or adding proper error handling for Slack API calls.-from slack_sdk.errors import SlackApiError
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
pyproject.toml(1 hunks)robusta_krr/core/runner.py(2 hunks)robusta_krr/main.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.11.9)
robusta_krr/core/runner.py
14-14: slack_sdk.errors.SlackApiError imported but unused
Remove unused import: slack_sdk.errors.SlackApiError
(F401)
🪛 Flake8 (7.2.0)
robusta_krr/core/runner.py
[error] 14-14: 'slack_sdk.errors.SlackApiError' imported but unused
(F401)
🔇 Additional comments (4)
pyproject.toml (1)
26-26: LGTM: Python version constraint update is appropriate.Extending the upper bound to
<=3.12.9allows compatibility with newer patch versions that include bug fixes and security updates without breaking changes.robusta_krr/main.py (1)
266-266: Excellent documentation improvement for Slack integration.The expanded help text clearly specifies the required OAuth scopes and setup requirements, which will help users properly configure their Slack bot integration. This aligns well with the API changes in the runner module.
robusta_krr/core/runner.py (2)
139-149: LGTM: Proper migration to Slack files_upload_v2 API.The transition from the deprecated
files_uploadtofiles_upload_v2API is correctly implemented. The new API requires channel IDs instead of names, which is properly handled by the new helper method.
151-182: Well-implemented channel ID resolution method.The
_resolve_slack_channel_idmethod properly handles:
- Channel ID detection (starts with 'C')
- Channel name normalization (removes '#' prefix)
- Pagination through Slack conversations API
- Comprehensive error messaging with troubleshooting guidance
The implementation is robust and follows Slack API best practices.
arikalon1
left a comment
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.
nice work
Fixes #373 and #408