-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Feat/provider pushbullet #27
Conversation
Eliminated the pushbullet.py dependency from requirements.txt to streamline the project and reduce unnecessary package management. This change helps maintain a cleaner environment and focuses on essential libraries.
Created a new module for the webhook site provider to support custom notification integrations. This addition allows for greater flexibility in notification delivery methods, enhancing the overall notification system.
Implemented Pushbullet notification provider to enable sending notifications across devices. This addition allows users to receive alerts formatted in Markdown, enhancing the notification experience. Configuration options include enabling the provider and including AI-generated summaries. - Integrated Pushbullet provider into the notification system - Added configuration options for API key and summary inclusion - Updated README and example config for new provider details
Reviewer's Guide by SourceryThis pull request introduces support for Pushbullet notifications. It includes the implementation of the Pushbullet provider, updates to the configuration schema and loading process, and modifications to the initialization process to incorporate the new provider. The changes also include updates to the documentation and example configuration file. Sequence diagram for initializing Pushbullet providersequenceDiagram
participant ConfigLoader
participant Config
participant PushbulletConfig
participant NotificationProvider
participant PushbulletProvider
ConfigLoader->Config: _init_config()
ConfigLoader->ConfigLoader: _load_pushbullet_config()
ConfigLoader->PushbulletConfig: Create PushbulletConfig
ConfigLoader->Config: pushbullet = PushbulletConfig
ConfigLoader->NotificationProvider: initialize_providers(config)
alt config.pushbullet.enabled
NotificationProvider->PushbulletProvider: PushbulletProvider(api_key, include_summary)
NotificationProvider->NotificationProvider: providers.append(PushbulletProvider)
end
Updated class diagram for configuration schemaclassDiagram
class Config
class WebhookSiteConfig {
enabled: bool
url: str
include_summary: bool
}
class PushbulletConfig {
enabled: bool
api_key: str
include_summary: bool
}
Config ..> WebhookSiteConfig : has
Config ..> PushbulletConfig : has
note for PushbulletConfig "New PushbulletConfig class added"
Class diagram for PushbulletProviderclassDiagram
class PushbulletProvider {
-api_key: str
-include_summary: bool
-base_url: str
-session: Session
+__init__(api_key: str, include_summary: bool)
+send(subject: str, message: str, summary: Optional[str]) : bool
}
class NotificationProvider{
<<Interface>>
+send(subject: str, message: str, summary: Optional[str]) : bool
}
PushbulletProvider --|> NotificationProvider : implements
note for PushbulletProvider "New PushbulletProvider class added"
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 @EvickaStudio - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a try-except block around the Pushbullet provider initialization to gracefully handle potential API key issues.
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.
# Send the request | ||
request_manager.update_headers(headers) | ||
response = self.session.post(f"{self.base_url}/pushes", json=payload) |
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.
suggestion: Use request-specific headers instead of mutating global session headers.
Passing the headers directly to the session.post call (e.g., using the 'headers' parameter) would prevent possible unintended side effects on the global session, which is especially important in concurrent environments.
# Send the request | |
request_manager.update_headers(headers) | |
response = self.session.post(f"{self.base_url}/pushes", json=payload) | |
# Send the request using request-specific headers | |
response = self.session.post(f"{self.base_url}/pushes", json=payload, headers=headers) |
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
This pull request introduces support for Pushbullet notifications in the project. The changes include updates to configuration files, the addition of a new Pushbullet provider, and modifications to the initialization process to incorporate the new provider.
New Pushbullet Notification Support:
README.md
: Added Pushbullet to the list of supported platforms and updated the example configuration section. [1] [2]example_config.ini
: Addedinclude_summary
setting under the Pushbullet section.src/core/config/loader.py
: Added methods to load Pushbullet configuration and updated the_init_config
and_load_provider_configs
methods to include Pushbullet. [1] [2] [3] [4]src/core/config/schema.py
: AddedPushbulletConfig
dataclass to define the configuration schema for Pushbullet.src/providers/notification/__init__.py
: Updated to initialize the Pushbullet provider if enabled and addedPushbulletProvider
to the list of notification providers. [1] [2] [3]src/providers/notification/pushbullet/provider.py
: Added the implementation of thePushbulletProvider
class for sending notifications via Pushbullet.Summary by Sourcery
Adds support for sending notifications via Pushbullet. This includes a new provider implementation, configuration settings, and updates to the initialization process.
New Features: