-
-
Notifications
You must be signed in to change notification settings - Fork 2
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
Destinations #290
Destinations #290
Conversation
Reviewer's Guide by SourceryThis PR improves error handling in the notification system and adds clarifying comments about the transcription service selection. The main changes involve wrapping the notification sending logic in a try-except block to gracefully handle missing destinations and adding documentation about the dependency requirements for different transcription services. Sequence diagram for sending notifications with error handlingsequenceDiagram
participant User
participant System
participant Apprise
User->>System: Trigger send_notifications
System->>System: Check destinations
alt Destination exists
System->>Apprise: Add notify_url
opt Attach audio
System->>Apprise: audio_notification
end
System->>Apprise: apobj.notify
else Destination missing
System->>User: Print error message
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @jquagga - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider catching specific exceptions (e.g. KeyError) instead of using a bare except clause to avoid masking unexpected errors
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Summary by Sourcery
Optimize GPU memory usage by conditionally importing torch only when using transformers and add error handling in the send_notifications function to handle missing destinations.
Enhancements:
Chores: