micro-analytics / micro-analytics-cli

Public analytics as a Node.js microservice. No sysadmin experience required! 📈
MIT License
734 stars 39 forks source link

use setTimeout instead of setImmediate #39

Closed abdulhannanali closed 7 years ago

abdulhannanali commented 7 years ago

setImmediate is a non standard feature and the behavior is non stable. In the src/utils.js I have replaced the setImmediate function call with an this alternative

setTimeout(async () => {
 await push()
}, 0)
codecov-io commented 7 years ago

Current coverage is 81.81% (diff: 100%)

Merging #39 into master will not change coverage

@@             master        #39   diff @@
==========================================
  Files             3          3          
  Lines            55         55          
  Methods           4          4          
  Messages          0          0          
  Branches         12         12          
==========================================
  Hits             45         45          
  Misses           10         10          
  Partials          0          0          

Powered by Codecov. Last update 09442c1...e6b06c3

abdulhannanali commented 7 years ago

I also would like to know if this locks functionality will work when we are using multiple processes. What are the alternatives in case we have multiple processes of one micro function running. Can we instead implement an increment functionality in the DB. Db such as redis provides us with this increment function which is atomic.

mxstbr commented 7 years ago

We've thought about moving the atomicity out to the database adapters, see #14 and related issues. Not sure yet if we'll go down that route, /cc @sean-roberts

sean-roberts commented 7 years ago

I don't think changing this is necessary. This is for sure standard in node. Maybe not in browsers according to mdn, but it's been in node quite a long time. :man_shrugging: As far as the locking convo, let's move that to a discussion ticket

sean-roberts commented 7 years ago

Want to give your thoughts on https://github.com/micro-analytics/micro-analytics/issues/40 ?

mxstbr commented 7 years ago

Closing this since we'll probably move forward with #40!

Thanks for taking the time to submit a PR and kicking off this discussion @abdulhannanali!