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

Script to check website status and send email to users if website is down #1398

Merged
merged 6 commits into from
Dec 10, 2024

Conversation

lukeckk
Copy link
Contributor

@lukeckk lukeckk commented Dec 2, 2024

Description

Created a file named 'checkWebsiteStatus.js' that contains the script to check website status and sends email to users when the website is down.

Partly Addresses #557

Type of change

  • Note merging this changes the database configuration.
  • This change requires a documentation update

Checklist

(Note what you have done by placing an "x" instead of the space in the [ ] so it becomes [x]. It is hoped you do all of them.)

  • I have followed the OED pull request ideas
  • I have removed text in ( ) from the issue request
  • You acknowledge that every person contributing to this work has signed the OED Contributing License Agreement and each author is listed in the Description section.

Limitations

Currently, I am unable to see if the email-sending part works. I verified that it prints 'The server is down' in the console when the website is down. I replaced some variables in docker-compose.yml with my emails and credentials, restarted the server and ran the script with command 'npm run status' but no email was received.

Could you run the script to see if it works on your end since your email is already in the database? Feel free give any feedback if you notice anything wrong.

The cron schedule is currently excluded for ease of testing.

Copy link
Member

@huss huss left a comment

Choose a reason for hiding this comment

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

Thanks to @lukeckk for working on this. It is a more comprehensive solution than I had thought about and that is nice. I know this is a draft PR but I'm providing some thoughts. I know there are a number of comments but that does not indicate that the work is not good and valuable. I just wanted to think about it holistically given the comprehensive nature of the solution. Know that I'm happy to discuss and get any thoughts you have.

src/server/services/checkWebsiteStatus.js Outdated Show resolved Hide resolved
src/server/services/checkWebsiteStatus.js Outdated Show resolved Hide resolved
src/server/services/checkWebsiteStatus.js Outdated Show resolved Hide resolved
src/server/services/checkWebsiteStatus.js Outdated Show resolved Hide resolved
src/server/services/checkWebsiteStatus.js Outdated Show resolved Hide resolved
src/server/services/checkWebsiteStatus.js Outdated Show resolved Hide resolved
src/server/services/checkWebsiteStatus.js Outdated Show resolved Hide resolved
src/server/services/checkWebsiteStatus.js Outdated Show resolved Hide resolved
src/server/services/checkWebsiteStatus.js Outdated Show resolved Hide resolved
lukeckk and others added 2 commits December 7, 2024 07:44
@huss
Copy link
Member

huss commented Dec 9, 2024

I'm leaving a comment that I realized the site person may not get notified if the site is down. If the server is in a bad state then the log email may not be sent. This is not different from other log emails but noting it here since it means the hope to notify the situation is lost. I'm okay with this situation and I think it may require creating code separate from OED to avoid this.

Copy link
Member

@huss huss left a comment

Choose a reason for hiding this comment

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

Thanks to @lukeckk for addressing the comments. Testing found it works as expected. I have made a few comments to finalize the code cleanup. I'm sorry we did not get this merged for your presentation but it should be close. Please let me know if I can help.

src/scripts/checkWebsiteStatusCron.bash Outdated Show resolved Hide resolved
src/scripts/checkWebsiteStatusCron.bash Show resolved Hide resolved
src/server/services/checkWebsiteStatus.js Outdated Show resolved Hide resolved
src/server/services/checkWebsiteStatus.js Outdated Show resolved Hide resolved
src/server/services/checkWebsiteStatus.js Outdated Show resolved Hide resolved
src/server/services/checkWebsiteStatus.js Outdated Show resolved Hide resolved
src/server/services/checkWebsiteStatus.js Outdated Show resolved Hide resolved
@lukeckk
Copy link
Contributor Author

lukeckk commented Dec 9, 2024

@huss Thank you very pointing out the comments and the unnecessary imports. I have made some updated according to your feedback. Regarding the indenting, could you fix it for me please? I gave it a few attempts but cant seem to get the spaces right. Thank you so much

- Fix formatting.
- uninstall node-cron so gone from package-lock.json
@huss huss marked this pull request as ready for review December 10, 2024 00:35
Copy link
Member

@huss huss left a comment

Choose a reason for hiding this comment

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

Thanks to @lukeckk for quickly addressing all the comments. Everything now looks good. I am also noting that you provided documentation to me and I plan to add it to the website. Congratulations on another accepted contribution.

@huss huss merged commit fa5e515 into OpenEnergyDashboard:development Dec 10, 2024
3 checks passed
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