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

Add signal API alerting #805

Closed
wants to merge 23 commits into from
Closed

Conversation

cooperose
Copy link

No description provided.

Comment on lines 61 to 65

# Step 3: Networking
If you have a firewall between the server and teslausb you will need to allow ports 22 and ICMP.
Port 22 is used for the rsync service
ICMP is used to verify the archive is reachable from teslausb
Copy link
Owner

Choose a reason for hiding this comment

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

This doesn't appear to have anything to do with adding Signal notifications?
(the bit about icmp is also no longer correct)

Copy link
Owner

Choose a reason for hiding this comment

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

You're still adding a new empty line to this file.
This file shouldn't be part of the PR at all.

Copy link
Owner

@marcone marcone left a comment

Choose a reason for hiding this comment

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

Please make sure SetupRSync.md isn't part of the PR, and that any shellcheck errors are addressed.

function send_signal () {
log "Sending Signal message."

curl -X POST -H "Content-Type: application/json" $SIGNAL_URL'/v2/send' \
Copy link
Owner

Choose a reason for hiding this comment

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

"$SIGNAL_URL/v2/send" is easier to read and should avoid the shellcheck error

@marcone marcone closed this Jan 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants