search

mrkamel / heartbeat

Use Heartbeat to monitor your Hetzner Failover IP and automatically switch to another server.
53 stars 13 forks source link

add dry option #10

Closed tpo closed 3 years ago

tpo commented 3 years ago

Hi,

this pull request should implement #4 - the dry option. It also improves a few other things as error messages and logs more things.

I have not added tests for the dry option, nor have I verified whether the modified error messages and the additional logs pass the tests. I'd be nice to have a note in the documentation on how these tests are supposed to be run, then I'd could have a look:

bash-5.1# rake test
/usr/local/bin/ruby -w -I"lib:lib" /usr/local/lib/ruby/gems/3.0.0/gems/rake-13.0.3/lib/rake/rake_test_loader.rb "test/failover_ip_test.rb" "test/hooks_test.rb" 

File does not exist: test/unit

rake aborted!
Command failed with status (1): [ruby -w -I"lib:lib" /usr/local/lib/ruby/gems/3.0.0/gems/rake-13.0.3/lib/rake/rake_test_loader.rb "test/failover_ip_test.rb" "test/hooks_test.rb" ]

Tasks: TOP => test
(See full trace by running task with --trace)

OT: I have tried to understand the failover_ip/ping_ip/ips: ping/target semantics by re-re-re-re-rereading the README and after that by studying the code, but I have not succeeded so far... a clearer README wrt to those semantics would be gold.

mrkamel commented 3 years ago

hi, thanks for your efforts. the project is a bit out of date, last update from 2015. have to bring it up to date first. created a PR to migrate to minitest. https://github.com/mrkamel/heartbeat/pull/11 Using that you should be able you to run the tests now.

tpo commented 3 years ago

@mrkamel thanks a lot for the minitest bits.

(updated comment because - see point 2. below - rebasing was actually easy :-) )

I've fixed stuff so that the tests run through plus i've added dry: true tests.

I have two branches in my fork:

  1. the master branch is based on your master (that is without minitest)
  2. the same_as_master_but_rebased_on_upstream_use_minitest_branch branch contains my changes rebased on top of your minitest branch
mrkamel commented 3 years ago

hi, could you maybe rebase this one then or create a new PR using the rebased version? thx alot

tpo commented 3 years ago

hi, could you maybe rebase this one then or create a new PR using the rebased version? thx alot

I will, however I think #13 should be resolved before I rebase - agreed?

mrkamel commented 3 years ago

I will, however I think #13 should be resolved before I rebase - agreed?

yup, fixed

tpo commented 3 years ago

A hi, could you maybe rebase this one then or create a new PR using the rebased version? thx alot

closed in favor of #15 that is based on the rebased branch of mine