Integrate Apprise notification service. #1589
Conversation
WalkthroughA new feature for notifications has been introduced, involving the addition of an Apprise API service. The changes include expanding the notification types to include 'apprise', adding a new environment variable for the API URL, and creating a new Docker service to support Apprise notifications. The modifications span across multiple files, introducing URL validation and preparing the infrastructure for a new notification method. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Server
participant AppriseAPI
Client->>Server: Create Notification
Server->>Server: Validate Notification
alt Valid Apprise URL
Server->>AppriseAPI: Send Notification
AppriseAPI-->>Server: Notification Status
Server-->>Client: Confirmation
else Invalid URL
Server-->>Client: Validation Error
end
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
Server/db/models/Notification.js (1)
Line range hint
1-93: Yo, we need some field validation based on notification type! 🍝The schema's got more loose ends than a bowl of spaghetti! We should validate that:
addressis required forphoneis required forsmstypeappriseUrlis required forapprisetypeHere's a clean implementation to add before the timestamps config:
NotificationSchema.pre('validate', function(next) { if (this.type === 'email' && !this.address) { this.invalidate('address', 'Email address is required for email notifications'); } if (this.type === 'sms' && !this.phone) { this.invalidate('phone', 'Phone number is required for SMS notifications'); } if (this.type === 'apprise' && !this.appriseUrl) { this.invalidate('appriseUrl', 'Apprise URL is required for Apprise notifications'); } next(); });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.env(1 hunks)Client/src/Components/Notifications/NotificationForm.jsx(1 hunks)Server/db/models/Notification.js(1 hunks)Server/services/NotificationService.js(1 hunks)docker-compose.yml(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- Client/src/Components/Notifications/NotificationForm.jsx
- Server/services/NotificationService.js
- .env
🧰 Additional context used
🪛 yamllint (1.35.1)
docker-compose.yml
[error] 8-8: no new line character at the end of file
(new-line-at-end-of-file)
[error] 8-8: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript-typescript)
Server/db/models/Notification.js
Outdated
| enum: ["email", "sms", "apprise"], | ||
| }, | ||
| appriseUrl: { | ||
| type: String, | ||
| validate: { | ||
| validator: function(v) { | ||
| return !this.type || | ||
| this.type !== 'apprise' || | ||
| /^[a-zA-Z]+:\/\//.test(v); | ||
| }, | ||
| message: 'Invalid Apprise URL format' | ||
| } |
There was a problem hiding this comment.
Yo dawg, we need to beef up that URL validation! 🍝
The current URL validation is weaker than mom's spaghetti. Here's what's making my palms sweaty:
- The regex
/^[a-zA-Z]+:\/\//only checks for scheme presence - No maximum length validation could lead to buffer overflow attacks
- No validation against malicious URLs
- Validation runs unnecessarily when type isn't 'apprise'
Here's a stronger implementation to make your knees stop being weak:
appriseUrl: {
type: String,
validate: {
validator: function(v) {
- return !this.type ||
- this.type !== 'apprise' ||
- /^[a-zA-Z]+:\/\//.test(v);
+ if (this.type !== 'apprise') return true;
+ if (!v || v.length > 2048) return false;
+ try {
+ const url = new URL(v);
+ return ['http:', 'https:'].includes(url.protocol);
+ } catch {
+ return false;
+ }
},
- message: 'Invalid Apprise URL format'
+ message: 'Invalid Apprise URL: Must be a valid HTTP(S) URL under 2048 characters'
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| enum: ["email", "sms", "apprise"], | |
| }, | |
| appriseUrl: { | |
| type: String, | |
| validate: { | |
| validator: function(v) { | |
| return !this.type || | |
| this.type !== 'apprise' || | |
| /^[a-zA-Z]+:\/\//.test(v); | |
| }, | |
| message: 'Invalid Apprise URL format' | |
| } | |
| enum: ["email", "sms", "apprise"], | |
| }, | |
| appriseUrl: { | |
| type: String, | |
| validate: { | |
| validator: function(v) { | |
| if (this.type !== 'apprise') return true; | |
| if (!v || v.length > 2048) return false; | |
| try { | |
| const url = new URL(v); | |
| return ['http:', 'https:'].includes(url.protocol); | |
| } catch { | |
| return false; | |
| } | |
| }, | |
| message: 'Invalid Apprise URL: Must be a valid HTTP(S) URL under 2048 characters' | |
| } |
There was a problem hiding this comment.
This suggestion might not be the best for Apprise URLs.
Apprise uses custom URL schemes for different services, for example:
discord://webhook_id/webhook_token
telegram://bottoken/ChatID
slack://tokenA/tokenB/tokenC
matrix://hostname?t=token
docker-compose.yml
Outdated
| apprise-api: | ||
| image: caronc/apprise-api:latest | ||
| container_name: apprise-api | ||
| ports: | ||
| - "8000:8000" | ||
| environment: | ||
| - APPRISE_STATELESS_MODE=yes | ||
| restart: unless-stopped |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Yo, this Docker config needs some sauce! 🍝
My sweaty palms spotted these issues in the Docker recipe:
- Using
latesttag is like gambling with mom's spaghetti - pin it to a specific version! - No health check means we can't tell if the service is ready to serve
- No resource limits could lead to container eating all resources
- Missing newline at EOF and has trailing spaces
Here's a beefed-up config that won't make your knees weak:
apprise-api:
- image: caronc/apprise-api:latest
+ image: caronc/apprise-api:v1.0.0 # Replace with actual stable version
container_name: apprise-api
ports:
- "8000:8000"
environment:
- APPRISE_STATELESS_MODE=yes
- restart: unless-stopped
+ restart: unless-stopped
+ healthcheck:
+ test: ["CMD", "curl", "-f", "http://localhost:8000/health"]
+ interval: 30s
+ timeout: 10s
+ retries: 3
+ deploy:
+ resources:
+ limits:
+ cpus: '0.50'
+ memory: 256M
+ reservations:
+ cpus: '0.25'
+ memory: 128M📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| apprise-api: | |
| image: caronc/apprise-api:latest | |
| container_name: apprise-api | |
| ports: | |
| - "8000:8000" | |
| environment: | |
| - APPRISE_STATELESS_MODE=yes | |
| restart: unless-stopped | |
| apprise-api: | |
| image: caronc/apprise-api:v1.0.0 # Replace with actual stable version | |
| container_name: apprise-api | |
| ports: | |
| - "8000:8000" | |
| environment: | |
| - APPRISE_STATELESS_MODE=yes | |
| restart: unless-stopped | |
| healthcheck: | |
| test: ["CMD", "curl", "-f", "http://localhost:8000/health"] | |
| interval: 30s | |
| timeout: 10s | |
| retries: 3 | |
| deploy: | |
| resources: | |
| limits: | |
| cpus: '0.50' | |
| memory: 256M | |
| reservations: | |
| cpus: '0.25' | |
| memory: 128M |
🧰 Tools
🪛 yamllint (1.35.1)
[error] 8-8: no new line character at the end of file
(new-line-at-end-of-file)
[error] 8-8: trailing spaces
(trailing-spaces)
![llamapreview[bot]](https://avatars.githubusercontent.com/in/1023541?s=60&v=4){kind=small}
There was a problem hiding this comment.
Auto Pull Request Review from LlamaPReview
1. Overview
1.1 PR Summary
- Business value and requirements alignment: This PR integrates the Apprise notification service into Checkmate, expanding its notification capabilities. This aligns with the business need to provide more versatile and robust notification options for monitoring server hardware, uptime, response times, and incidents.
- Key components modified: The PR modifies the notification schema, service logic, and configuration files to include Apprise notifications. It also updates the Docker configuration to include the Apprise API service.
- Impact assessment: The integration of Apprise notifications enhances the system's ability to alert users through various channels, improving overall monitoring effectiveness. However, it introduces new dependencies and potential points of failure that need careful management.
- System dependencies and integration impacts: The addition of the
apprise-apiservice impacts the deployment architecture and requires careful configuration management. The system now depends on the availability and reliability of the Apprise API.
1.2 Architecture Changes
- System design modifications: The introduction of Apprise notifications adds a new layer of abstraction for notification delivery. The
Notificationmodel is extended to include Apprise-specific fields, and theNotificationServiceis expected to handle the logic for sending Apprise notifications. - Component interactions: The core interaction point is within the
NotificationService.js, where the logic for sending notifications will be extended to support Apprise. TheNotificationmodel is also a critical interaction point, with the addition of theappriseUrlfield. - Integration points: The
.envanddocker-compose.ymlchanges indicate a new service dependency and its configuration. The client-side changes inNotificationForm.jsxsuggest UI updates for configuring Apprise notifications.
2. Detailed Technical Analysis
2.1 Code Logic Deep-Dive
Core Logic Changes
- Server/db/models/Notification.js - NotificationSchema
- Submitted PR Code:
@@ -1,21 +1,32 @@
import mongoose from "mongoose";
const NotificationSchema = mongoose.Schema(
{
monitorId: {
type: mongoose.Schema.Types.ObjectId,
ref: "Monitor",
immutable: true,
},
type: {
type: String,
- enum: ["email", "sms"],
+ enum: ["email", "sms", "apprise"],
+ },
+ appriseUrl: {
+ type: String,
+ validate: {
+ validator: function(v) {
+ return !this.type ||
+ this.type !== 'apprise' ||
+ /^[a-zA-Z]+:\/\//.test(v);
+ },
+ message: 'Invalid Apprise URL format'
+ }
},
address: {
type: String,
},
phone: {
type: String,
- },
+ },
alertThreshold: {
type: Number,
default: 5,- Analysis:
- The addition of
appriseUrland its basic validation is noted. However, the current validation logic has a potential flaw. Ifthis.typeis undefined, the validation will pass regardless of theappriseUrlformat. This could lead to invalid Apprise URLs being stored if thetypeis added later or through a different update path. - The regex
/^[a-zA-Z]+:\/\//checks for a basic protocol but is overly simplistic and doesn't account for various valid URL formats or potential security concerns. This could lead to unexpected behavior or even security vulnerabilities if a crafted URL is used.
- The addition of
- LlamaPReview Suggested Improvements:
@@ -8,10 +8,12 @@
immutable: true,
},
type: {
- type: String,
+ type: String,
+ required: true, // Ensure type is always defined
enum: ["email", "sms", "apprise"],
},
appriseUrl: {
+ required: function() { return this.type === 'apprise'; }, // Ensure appriseUrl is required when type is apprise
type: String,
validate: {
validator: function(v) {
this.type !== 'apprise' ||
/^(?:[a-z]+:\/\/)?(?:[\w-]+\.)+[\w-]+(?:[\w.,@?^=%&:/~+#-]*[\w@?^=%&/~+#-])?$/.test(v);
},
message: 'Invalid Apprise URL format'
}-
Improvement rationale:
- Technical benefits: Ensures that the
typefield is always defined, preventing bypass ofappriseUrlvalidation. MakesappriseUrlmandatory when the notification type is 'apprise', enforcing data integrity. Uses a more robust regular expression to validate theappriseUrl, reducing the risk of malformed URLs. - Risk assessment: Reduces the risk of storing invalid Apprise URLs, which could lead to notification failures. Improves the reliability of the Apprise integration by reducing the likelihood of errors caused by invalid URLs.
- Technical benefits: Ensures that the
-
Server/services/NotificationService.js - (Potentially new logic for handling Apprise notifications)
- Analysis:
- The PR description mentions adding Apprise integration, and the
Notificationmodel now includesappriseUrl. However, the provided file changes do not include any modifications to this file. This is a significant oversight. The core logic for sending notifications via Apprise would reside here. Without seeing this code, it's impossible to assess how Apprise notifications are being handled, including error handling, API interactions, and potential security concerns related to sending data to the Apprise API. - Assuming there is code in
NotificationService.jsto handle Apprise, several questions arise that the initial review doesn't address because the code is missing:- How is the
APPRISE_API_URLenvironment variable being used? Is it being accessed securely? - What HTTP client is being used to interact with the Apprise API? Are there appropriate timeouts and error handling implemented for network requests?
- How are different Apprise notification types (based on the URL scheme) being handled?
- Is there any logging or monitoring of Apprise notification attempts and their success/failure?
- How is authentication (if required by the Apprise instance) being handled?
- How is the
- The PR description mentions adding Apprise integration, and the
- LlamaPReview Suggested Improvements: (Cannot provide specific code improvements without the actual code, but highlighting key considerations)
- Analysis:
// Example of potential Apprise notification sending logic (ASSUMPTION - code is missing from PR details)
async sendAppriseNotification(notification) {
try {
const response = await fetch(`${process.env.APPRISE_API_URL}/notify`, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
},
body: JSON.stringify({
urls: [notification.appriseUrl],
body: `Alert for monitor: ${notification.monitorId}`, // Example payload
}),
});
if (!response.ok) {
console.error(`Apprise notification failed: ${response.status} - ${await response.text()}`);
// Implement retry logic or error tracking
}
} catch (error) {
console.error('Error sending Apprise notification:', error);
// Handle network errors
}
}- Improvement rationale:
- Technical benefits: Illustrates the kind of logic expected in
NotificationService.js. Highlights the need for error handling, network request management, and secure access to configuration. - Risk assessment: Points out potential areas of failure and security concerns if the Apprise integration is not implemented carefully. The missing code represents a significant risk as the actual implementation is unknown.
- Technical benefits: Illustrates the kind of logic expected in
2.2 Implementation Quality
- Code organization and structure: The PR is well-organized, with changes isolated to specific files related to the notification system. The addition of the
appriseUrlfield and its validation logic in theNotificationmodel is clear and concise. - Design patterns usage: The use of Mongoose schemas for data validation is a good practice. However, the current validation logic for
appriseUrlneeds improvement to ensure data integrity and security. - Error handling approach: The error handling for Apprise notifications is not evident in the provided code. It is crucial to implement robust error handling to manage failures in sending notifications via the Apprise API.
- Resource management: The introduction of the
apprise-apiservice in the Docker configuration is well-managed. However, the PR lacks details on how this service will be monitored and managed in production.
3. Critical Findings
3.1 Potential Issues
-
🔴 Critical Issues
- Missing Apprise Notification Logic: The core logic for sending Apprise notifications is missing from the
NotificationService.jsfile. This is a critical gap that needs to be addressed to ensure the Apprise integration is functional and reliable.- Impact: Without the core logic, the Apprise notification feature cannot be tested or deployed, leading to potential failures in notification delivery.
- Recommendation: Implement the missing logic in
NotificationService.jsto handle Apprise notifications, including error handling, API interactions, and secure access to configuration.
- Missing Apprise Notification Logic: The core logic for sending Apprise notifications is missing from the
-
🟡 Warnings
- Validation Logic for
appriseUrl: The current validation logic forappriseUrlis overly simplistic and does not account for various valid URL formats or potential security concerns.- Potential risks: This could lead to unexpected behavior or even security vulnerabilities if a crafted URL is used.
- Suggested improvements: Use a more robust regular expression to validate the
appriseUrl, reducing the risk of malformed URLs.
- Validation Logic for
3.2 Code Quality Concerns
- Maintainability aspects: The PR introduces a new dependency on the
apprise-apiservice, which needs to be carefully managed. The current validation logic forappriseUrlcould lead to maintainability issues if invalid URLs are stored. - Readability issues: The PR is generally readable, but the missing logic in
NotificationService.jsmakes it difficult to understand the complete implementation of the Apprise integration. - Performance bottlenecks: The performance implications of adding another notification mechanism need to be considered, especially under high load.
4. Security Assessment
- Authentication/Authorization impacts: The PR does not introduce any changes to authentication or authorization mechanisms. However, the interaction with the Apprise API may require authentication, which needs to be securely managed.
- Data handling concerns: The
appriseUrlcould potentially contain sensitive information or be a vector for malicious activity if not handled correctly. The security implications of communicating with an external service need to be assessed. - Input validation: The current validation logic for
appriseUrlis not sufficient to prevent potential security vulnerabilities. A more robust validation is required. - Security best practices: Ensure that the
APPRISE_API_URLenvironment variable is accessed securely. Implement appropriate timeouts and error handling for network requests to the Apprise API. - Potential security risks: The missing logic in
NotificationService.jsrepresents a significant risk as the actual implementation is unknown. The current validation logic forappriseUrlcould lead to security vulnerabilities if a crafted URL is used. - Mitigation strategies: Implement robust error handling and secure access to configuration in
NotificationService.js. Use a more comprehensive regular expression to validate theappriseUrl. - Security testing requirements: Conduct thorough security testing of the Apprise integration, including testing for injection attacks and ensuring secure communication with the Apprise API.
5. Testing Strategy
5.1 Test Coverage
- Unit test analysis: The PR lacks unit tests for the Apprise notification logic. Unit tests are crucial to ensure the functionality and reliability of the Apprise integration.
- Integration test requirements: Integration tests are needed to verify the communication with the
apprise-apiservice and the overall notification process. - Edge cases coverage: Test different Apprise URL schemes and configurations to ensure the system handles various scenarios correctly.
5.2 Test Recommendations
Suggested Test Cases
// Example unit test for Apprise notification logic (ASSUMPTION - code is missing from PR details)
describe('sendAppriseNotification', () => {
it('should send a notification to the Apprise API', async () => {
// Mock the fetch function
global.fetch = jest.fn(() =>
Promise.resolve({
ok: true,
json: () => Promise.resolve({ success: true }),
})
);
const notification = {
monitorId: '12345',
type: 'apprise',
appriseUrl: 'http://example.com',
};
await sendAppriseNotification(notification);
expect(fetch).toHaveBeenCalledWith(
'http://localhost:8000/notify',
expect.objectContaining({
method: 'POST',
headers: {
'Content-Type': 'application/json',
},
body: JSON.stringify({
urls: ['http://example.com'],
body: 'Alert for monitor: 12345',
}),
})
);
});
it('should handle errors when sending a notification', async () => {
// Mock the fetch function to simulate an error
global.fetch = jest.fn(() =>
Promise.reject(new Error('Network error'))
);
const notification = {
monitorId: '12345',
type: 'apprise',
appriseUrl: 'http://example.com',
};
await expect(sendAppriseNotification(notification)).rejects.toThrow('Network error');
});
});- Coverage improvements: Ensure that the Apprise notification logic is thoroughly tested, including successful sending and error handling scenarios.
- Performance testing needs: Conduct performance testing to assess the impact of adding another notification mechanism, especially under high load.
6. Documentation & Maintenance
- Documentation updates needed: Update the API, architecture, and configuration documentation to include details about the Apprise notification integration.
- Long-term maintenance considerations: Monitor the availability and reliability of the
apprise-apiservice. Implement logging and monitoring for Apprise notification attempts and their success/failure. - Technical debt and monitoring requirements: Address the technical debt introduced by the missing logic in
NotificationService.js. Implement robust monitoring to detect and handle failures in the Apprise integration.
7. Deployment & Operations
- Deployment impact and strategy: The introduction of the
apprise-apiservice requires updates to the deployment process. Ensure that the service is correctly configured and monitored in production. - Key operational considerations: Monitor the performance and reliability of the Apprise integration. Implement logging and alerting for any failures in sending Apprise notifications.
8. Summary & Recommendations
8.1 Key Action Items
- Implement the missing logic in
NotificationService.jsto handle Apprise notifications, including error handling, API interactions, and secure access to configuration. - Improve the validation logic for
appriseUrlto use a more robust regular expression and ensure data integrity and security. - Conduct thorough security testing of the Apprise integration, including testing for injection attacks and ensuring secure communication with the Apprise API.
- Update the documentation to include details about the Apprise notification integration.
8.2 Future Considerations
- Technical evolution path: Continuously monitor and improve the Apprise integration based on feedback and performance metrics.
- Business capability evolution: Explore additional notification channels and features to enhance the system's monitoring capabilities.
- System integration impacts: Ensure that the Apprise integration is seamlessly integrated with existing notification channels and does not introduce conflicts or performance bottlenecks.
💡 Help Shape LlamaPReview
How's this review format working for you? Vote in our Github Discussion Polls to help us improve your review experience!
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
docker-compose.yml (1)
14-21:⚠️ Potential issueThese resource limits are making my knees weak! 💪
The memory reservation is like a tiny plate of spaghetti - might be too small for Apprise's appetite. Consider bumping it up:
reservations: cpus: '0.25' - memory: 128M + memory: 192MAlso, there's vomit on these trailing spaces already! Let's clean that up and add a newline at EOF.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 21-21: no new line character at the end of file
(new-line-at-end-of-file)
[error] 21-21: trailing spaces
(trailing-spaces)
🧹 Nitpick comments (1)
docker-compose.yml (1)
4-5: Heads up on that port exposure, homie! 🔓That port 8000 is wide open like mom's kitchen - might wanna lock it down to localhost if you're not serving external requests:
ports: - - "8000:8000" + - "127.0.0.1:8000:8000"
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docker-compose.yml(1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
docker-compose.yml
[error] 21-21: no new line character at the end of file
(new-line-at-end-of-file)
[error] 21-21: trailing spaces
(trailing-spaces)
🔇 Additional comments (3)
docker-compose.yml (3)
1-3: Yo dawg, that container config is lookin' fresh! 🍝The version pinning to 1.2.0 shows you ain't playing with fire like using
latest- that's some good pasta right there!
6-8: That stateless mode is straight fire! 🔥Setting
APPRISE_STATELESS_MODE=yesis clean like mom's kitchen - perfect for containerized deployments!
9-13: 🛠️ Refactor suggestionYo, that health check endpoint looks sus! 🤔
My sweaty palms noticed you're checking the root endpoint
/instead of a dedicated health endpoint. That's like using the front door to check if the kitchen's clean!healthcheck: - test: ["CMD", "curl", "-f", "http://localhost:8000/"] + test: ["CMD", "curl", "-f", "http://localhost:8000/health"]Likely invalid or redundant comment.
There was a problem hiding this comment.
Looks like a good start 👍
There's a .env that has been comitted, we should pop off that commit so it is not included in our repo history.
I'm not 100% clear on the requirements for this PR, so I'm not sure how the docker compose file fits into things just yet. Will clarify requirements and revisit this!
| @@ -0,0 +1 @@ | |||
| APPRISE_API_URL=http://localhost:8000 | |||
There was a problem hiding this comment.
.env files shouldn't be committed, best to remove this commit entirely
| if (!v || v.length > 2048) return false; | ||
|
|
||
| // Apprise URL format: scheme://[user:pass@]host[:port]/path | ||
| const appriseUrlRegex = /^[a-zA-Z]+:\/\/([^:\s]+:[^@\s]+@)?[^\s\/:]+(:\d+)?(\/[^\s]*)?$/; |
There was a problem hiding this comment.
This regex will also allow for invalid Apprise URLs, this will match for any alphabetical string for schema, like http:// or abcd://
If we want to strictly verify Apprise URLs only, we'll have to match for all the specific protocols.
Otherwise we may as well just verify that this is a string and leave the responsibility for a valid URL with the user.
| memory: 256M | ||
| reservations: | ||
| cpus: '0.25' | ||
| memory: 128M |
There was a problem hiding this comment.
I can see this docker compose file is meant to spin up an instance of Apprise, but how does this fit into the application structure?
I don't believe we should have the responsibility of maintaining a docker compose file that runs an instance of Apprise for end users, I think that's their responsibility. I believe it is sufficient that we provide the integration 🤔
I will clarify the requirements, but if it turns out we do need to provide an instance of Apprise then this should be baked into our existing docker compose files.
| @@ -8,7 +8,21 @@ const NotificationSchema = mongoose.Schema( | |||
| }, | |||
| type: { | |||
| type: String, | |||
| enum: ["email", "sms"], | |||
| enum: ["email", "sms", "apprise"], | |||
|
Going with a different approach |
Describe your changes
Add Apprise notification service integration
Issue number
#1545
Please ensure all items are checked off before requesting a review: