yegor256 / sixnines

Website Availability Monitor: add your website to our dashboard and get 24x7 monitoring of its availability (and a badge!)
https://www.sixnines.io
MIT License
72 stars 10 forks source link

pulse #85

Closed pdacostaporto closed 7 years ago

pdacostaporto commented 7 years ago

Solves #36

0crat commented 7 years ago

Job gh:yegor256/sixnines#85 is in scope.

0crat commented 7 years ago

+15 points just awarded to @pdacostaporto, total is +135.

codecov-io commented 7 years ago

Codecov Report

Merging #85 into master will increase coverage by 1.84%. The diff coverage is 52.25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #85      +/-   ##
==========================================
+ Coverage   40.28%   42.12%   +1.84%     
==========================================
  Files          19       23       +4     
  Lines         633      743     +110     
==========================================
+ Hits          255      313      +58     
- Misses        378      430      +52
Impacted Files Coverage Δ
test/test_base.rb 47.36% <22.22%> (-22.64%) :arrow_down:
sixnines.rb 35.13% <50%> (+0.32%) :arrow_up:
test/test_counted_resource.rb 55.55% <55.55%> (ø)
test/test_ping_count.rb 57.5% <57.5%> (ø)
objects/counted_resource.rb 61.53% <61.53%> (ø)
objects/endpoint.rb 36% <66.66%> (+1.75%) :arrow_up:
objects/base.rb 55.55% <75%> (+3.38%) :arrow_up:
objects/ping_count.rb 77.77% <77.77%> (ø)
test/test_endpoint.rb 27.5% <9.09%> (-6.99%) :arrow_down:
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 4768d8d...cb7dde8. Read the comment docs.

yegor256 commented 7 years ago

@pdacostaporto I didn't understand why do we need a DynamoDB table for that? We need just one counter, to count how many pings we did after the restart of the app. We can use just a simple global variable for that. We don't need to count per endpoint, since we already count that. All we need is a "total since reboot". Or I didn't understand your code?

pdacostaporto commented 7 years ago

@yegor256 well... i had some thoughts which led to the table idea. I automatically tend to avoid globals in code (is that what you mean?) and singletons to avoid concurrency issues, coupling, mutability and those evil things (although maybe there's a way to avoid them, I'm not sure). DynamoDB handles concurrency automatically, so it was possible to store the count there without those issues. Maybe concurrency its not an issue but maybe I'm not sure because I'm not sure about how this application works. I also found some examples of visit counters stored in databases, so it didn't look like a bad idea.

It's possible that you didn't understand the code. It works with an only global counter. Yes: there's a whole table for just one record; couldn't find out other way to store it.

Anyway, should I use a global variable in the code then?

yegor256 commented 7 years ago

@pdacostaporto let's go with a global variable for now, of class, say TotalPings. An instance of this class we will create at sixnines.rb initialization method and then pass to the endpoints. How to handle the concurrency is a good question. I'm not really an expert in Ruby in this area. Maybe there are some gems for that?

pdacostaporto commented 7 years ago

@yegor256 if the endpoints are responsible of incrementing the counter, it should be a mutable object, right?

I'm thinking about two options: -AtomicFixNum from Concurrent Ruby gem. -Thread::Mutex. The first option looks easier, but with more magic. What do you think?

pdacostaporto commented 7 years ago

I did it with AtomicFixnum in #87. If it's OK, I'll close this.

yegor256 commented 7 years ago

@pdacostaporto yes, #87 seems correct