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

hardcoded configs specific to author's own network/environment #15

Open
dotben opened this issue Aug 22, 2021 · 6 comments
Open

hardcoded configs specific to author's own network/environment #15

dotben opened this issue Aug 22, 2021 · 6 comments

Comments

@dotben
Copy link

dotben commented Aug 22, 2021

There are a number of hardcoded configurations and containers launched that appear to be specific to the author's own home environment and unrelated to internet monitoring.

For example, in prometheus/pinghosts.yml a number of local resources are harcoded to ping local resources unlikely to exit on any other environment. When the docker compose yaml runs, it creates a whole container for some kind of Sonos data exportation that again is specific to the author's environment and isn't relevant to internet monitoring.

Totally appreciate the author taking the time to produce this repo, but wondering if he could remove the cruft that at best is wasted resource and at worst might impair system performance.

Thank you

@Quinten0508
Copy link

+1, the sonos stuff is not relevant in the slightest and disabling the network connection does not break/change the container's workings in the slightest.

@maxandersen
Copy link
Owner

pull requests welcome

@dotben
Copy link
Author

dotben commented Oct 6, 2022

I ended up deciding not to use this option, mostly because from the issue above I didn't have confidence it was properly built and configured to be run on any arbitrary install. I accept where @maxandersen you are coming from with wanting the community to make PRs but the reality is you know best what code is core business logic and what is system cruft from your own install, and you might foster a larger community to help with future development if you removed this yourself.

As it stands I'm not a vested member of this community as I don't use the package and so v unlikely to put together a PR.

It's chicken and egg.

@Quinten0508
Copy link

Decided to patch this problem and a ton of other stuff up myself for the time being. @maxandersen take a look if you'd want a pull req as-is and let me know :) The commit messages should mostly speak for themselves on what has been changed.

If you'd want to keep the Sonos stuff and whatnot for your own home then it might be worth splitting the project into multiple branches and just merging changes from master to e.g. private or home.

https://github.com/Quinten0508/internet-monitoring

@Quinten0508
Copy link

I'm not super experienced with Docker nor have I read through all of the code for e.g. Sonos integration. Might be worth doing a cleanup and removing now unnecessary code from other parts of the project. However, the docker container itself is gone which should mean little to no performance is lost and this would be mostly cleaning stuff up.

@maxandersen
Copy link
Owner

I dont need the Sonos specific setup anymore. Feel free to submit PR that leaves it out to make it more generic.

GeorgeDewar pushed a commit to GeorgeDewar/internet-monitoring that referenced this issue Aug 12, 2023
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

No branches or pull requests

3 participants