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

[FEATURE] Option for Ping-Only Status Checks #233

Closed
wants to merge 14 commits into from

Conversation

Lissy93
Copy link
Owner

@Lissy93 Lissy93 commented Sep 18, 2021

Medium Lissy93 /FEATURE/ping-only-status-check-option → Lissy93/dashy Commits: 14 | Files Changed: 18 | Additions: 125 ✨ Feature 🚫 Merge Conflicts

Category: Feature

Overview
Several users have faced issues either with certain services having failing status checks, or being unable to check the status of web services that don't render any content (such as game servers, mail servers, file servers, etc). This is because the default status check option works by sending an empty GET request to the URL, which obviously cannot work for services that are not designed to accept these requests.

This PR introduces a new option, statusCheckPingOnly. If this is set to true for a given service, then just a traditional ping will be sent to that IP address.

Issue Number: #181 (Kinda sort of related)

New Vars
statusCheckPingOnly (boolean Optional) If enabled, instead of sending a HTTP request, this option will use a simple ping to your app

Screenshot N/A

Code Quality Checklist (Please complete)

  • All changes are backwards compatible
  • All lint checks and tests are passing
  • There are no (new) build warnings or errors
  • (If a new config option is added) Attribute is outlined in the schema and documented
  • (If a new dependency is added) Package is essential, and has been checked out for security or performance
  • Bumps version, if new feature added

@netlify
Copy link

netlify bot commented Sep 18, 2021

✔️ Deploy Preview for dashy-dev ready!

🔨 Explore the source changes: 56d4b44

🔍 Inspect the deploy log: https://app.netlify.com/sites/dashy-dev/deploys/61453b53e19b9c0007e0497a

😎 Browse the preview: https://deploy-preview-233--dashy-dev.netlify.app

@liss-bot
Copy link
Collaborator

liss-bot commented Sep 18, 2021

Hi Lissy93! Thank you for contributing to Dashy! ✨

When making changes to the documentation, be sure to double check that:

  • Link and images URLs properly resolve
  • Your spelling and grammar is correct
  • Any markdown formatting is valid

You're making changes to the main server entry point. Please test the app thoroughly, as any misconfigurations here may cause the app to stop functioning.

When updating dependencies, take a moment to verify that there are not security issues associated with any added or modified packages. If adding a new dependency, ensure that it is totally necessary, and check the packages size is not too large, as this will increase overall bundle size.

Don't forget to verify they the config validator script responds correctly to your new attribute.


I'm a bot, and this is an automated comment 🤖

@liss-bot
Copy link
Collaborator

yarn.lock changes

Click to toggle table visibility
Name Status Previous Current
nan DOWNGRADED 2.15.0 2.14.2
net-ping ADDED - 1.2.3
raw-socket ADDED - 1.7.0

@viezly
Copy link

viezly bot commented Sep 18, 2021

This pull request is split into 4 parts for easier review.
👀 Review pull request on Viezly

Changed files are located in these folders:

  • /
  • docs
  • src/components
  • src/utils

@Lissy93 Lissy93 added the ✨ New Feature [PR] Contains implementation of a new feature label Sep 18, 2021
Repository owner deleted a comment from liss-bot Sep 18, 2021
Repository owner deleted a comment from liss-bot Sep 18, 2021
@Lissy93
Copy link
Owner Author

Lissy93 commented Sep 18, 2021

Hmm, so it turns out that net-ping requires Python to work, and I do not want to add this to the Docker container, since it adds additional weight.

I need to either find a cross-platform method of pinging an endpoint, or figure out how to sort the net-ping node-gyp issue. I really should have looked into that before building this feature out 🤦

Repository owner deleted a comment from liss-bot Sep 18, 2021
@Lissy93
Copy link
Owner Author

Lissy93 commented Sep 18, 2021

Actually including: apk add --update python make g++ && rm -rf /var/cache/apk/* fixed it.
However I am worried that less experienced users, running without Docker may get confused by this. Plus I want to try and keep the container as small as possible.

The Docker image would be a lot more light-weight if I re-wrote the server-side stuff in Go instead of Node. And in doing so, the ping functionality would be easy. But since that'd be a big task, it will have to wait till I have some spare time.

@liss-bot liss-bot added the 🚫 Merge Conflicts [PR] Submitted code needs rebasing label Oct 5, 2021
@Lissy93 Lissy93 closed this Oct 31, 2021
@masterwishx
Copy link

Sorry for missunderstanding ,so what should we do ?

run in consol ?
apk add --update python make g++ && rm -rf /var/cache/apk/*e

@Lissy93
Copy link
Owner Author

Lissy93 commented Jun 9, 2022

No, because this feature was never merged (it's closed). So it's not possible to use it. I mentioned this in your other thread.

@masterwishx
Copy link

you said i can somehow to ping no ?

@Lissy93
Copy link
Owner Author

Lissy93 commented Jun 9, 2022

I said it's been discussed before, and isn't possible in dashy. You'll need to write a script yourself. But I'm 90% sure you can get status checking working by following the docs.

Feel like this is goning round in circles now.

@masterwishx
Copy link

OK Thanks a lot i will try ..

asterling8516 pushed a commit to asterling8516/dashy that referenced this pull request Nov 23, 2023
Signed-off-by: Bjorn Lammers <walkxnl@gmail.com>
asterling8516 pushed a commit to asterling8516/dashy that referenced this pull request Nov 23, 2023
Closes Lissy93#233

Signed-off-by: Bjorn Lammers <walkxnl@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚫 Merge Conflicts [PR] Submitted code needs rebasing ✨ New Feature [PR] Contains implementation of a new feature 🟨 PR - Medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants