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

Monitoring install scripts #3985

Merged
merged 5 commits into from Feb 20, 2020
Merged

Conversation

@freimair
Copy link
Member

freimair commented Feb 18, 2020

Completes first part of #3916.

  • cleans up the seednode install script
  • install script for monitoring servers with collectd
  • install script for the networksize metric

I tested the script for general monitoring using a fresh ubuntu 18.04 LTS installation. I could not test the script for the networksize metric as thoroughly, as there is no install script for a price node available yet.

@freimair freimair requested a review from cbeams as a code owner Feb 18, 2020
@freimair freimair requested a review from wiz Feb 18, 2020
@ripcurlx

This comment has been minimized.

Copy link
Member

ripcurlx commented Feb 18, 2020

@freimair Could you have a look at https://app.codacy.com/gh/bisq-network/bisq/pullRequest?prid=5023490. If there are false positives please let me know and I'll adapt the checks. Thanks!

@freimair freimair force-pushed the freimair:monitoring_install_script branch 2 times, most recently from 79eb307 to 3d4ba9a Feb 18, 2020
@freimair

This comment has been minimized.

Copy link
Member Author

freimair commented Feb 18, 2020

@freimair Could you have a look at https://app.codacy.com/gh/bisq-network/bisq/pullRequest?prid=5023490. If there are false positives please let me know and I'll adapt the checks. Thanks!

you beat me to that. I already fixed the issues. And nope, the checks seem fine. You get these issues if you copy and paste your way through life.

Is there a way to use codacy before creating the PR? (because no force-pushes, no fix commits, yadda yadda...)

@ripcurlx

This comment has been minimized.

Copy link
Member

ripcurlx commented Feb 18, 2020

Is there a way to use codacy before creating the PR? (because no force-pushes, no fix commits, yadda yadda...)

The only way would be if you enable it on your fork yourself to run on every branch you push.

@freimair freimair force-pushed the freimair:monitoring_install_script branch 2 times, most recently from 69df120 to c090416 Feb 18, 2020
@wiz

This comment has been minimized.

Copy link
Member

wiz commented Feb 19, 2020

@ripcurlx

This comment has been minimized.

Copy link
Member

ripcurlx commented Feb 19, 2020

@freimair see this line for a workaround for the codacy issue https://github.com/bisq-network/bisq-explorer/blob/master/install_bsq_explorer_debian.sh#L56

If you guys think this Codacy remark is not correct, then I can just deactivate this single check.

@freimair freimair force-pushed the freimair:monitoring_install_script branch from c090416 to 0d17bb6 Feb 19, 2020
@freimair

This comment has been minimized.

Copy link
Member Author

freimair commented Feb 19, 2020

should all be good now. My bad.

Copy link
Member

cbeams left a comment

I was pinged here for a code review because I'm the pricenode code owner, but the changes there are minimal and do not affect production code, so I'm giving a my OK from the pricenode side, but am abstaining from a larger review.

monitor/install_collectd_debian.sh Outdated Show resolved Hide resolved
monitor/install_collectd_debian.sh Outdated Show resolved Hide resolved
monitor/install_collectd_debian.sh Outdated Show resolved Hide resolved
monitor/install_collectd_debian.sh Outdated Show resolved Hide resolved
monitor/install_collectd_debian.sh Show resolved Hide resolved
monitor/install_collectd_debian.sh Show resolved Hide resolved
monitor/install_collectd_debian.sh Outdated Show resolved Hide resolved
freimair and others added 2 commits Feb 20, 2020
Co-Authored-By: wiz <j@wiz.biz>
Co-Authored-By: wiz <j@wiz.biz>
monitor/install_collectd_debian.sh Outdated Show resolved Hide resolved
monitor/install_collectd_debian.sh Outdated Show resolved Hide resolved
monitor/install_collectd_debian.sh Outdated Show resolved Hide resolved
monitor/install_collectd_debian.sh Outdated Show resolved Hide resolved
Co-Authored-By: wiz <j@wiz.biz>
@wiz
wiz approved these changes Feb 20, 2020
Copy link
Member

wiz left a comment

Tested 👍 on fresh Ubuntu 18 install using seednode installer + collectd installer

Copy link
Member

ripcurlx left a comment

utACK

Based on #3985 (review)

@ripcurlx ripcurlx merged commit 7943026 into bisq-network:master Feb 20, 2020
2 checks passed
2 checks passed
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ripcurlx ripcurlx added this to the v1.2.8 milestone Feb 26, 2020
@freimair freimair mentioned this pull request Mar 11, 2020
@ripcurlx ripcurlx mentioned this pull request Mar 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.