search

micro-analytics / micro-analytics-cli

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

Add db adapters #14

Closed mxstbr closed 7 years ago

mxstbr commented 7 years ago

Closes #8.

sean-roberts commented 7 years ago

This sounds good to me 👍🏼

codecov-io commented 7 years ago

Current coverage is 87.50% (diff: 86.00%)

Merging #14 into master will decrease coverage by 5.65%

@@             master        #14   diff @@
==========================================
  Files             4          5     +1   
  Lines            73        104    +31   
  Methods          13         23    +10   
  Messages          0          0          
  Branches         14         18     +4   
==========================================
+ Hits             68         91    +23   
- Misses            5         13     +8   
  Partials          0          0

Powered by Codecov. Last update 6aebd25...f29ae12

relekang commented 7 years ago

I cannot update the checkboxes in the description, but getAll is promised based now and tests work.

If you want to to try it with redis 

npm i relekang/micro-analytics-adapter-redis # will publish this when i have implemented configuration for the redis connection
DB_ADAPTER=redis npm run start

@mxstbr with point two, do you want this rebased on top of #4?

mxstbr commented 7 years ago

No, definitely not. I want the filtering to live in the database adapters, because e.g. Redis might have built-in filtering that's faster than getting out everything and then filtering the results!

relekang commented 7 years ago

@mxstbr Not sure what happened to the tests after merging you changes, but will look on that and the before after filtering later today. The spec on filtering in your comment looks good 👌

sean-roberts commented 7 years ago

A section on "Adding New DB Adaptors" is probably warranted too. Define the interface and options that are passed and the expectations from the combinations from an enduser perspective

relekang commented 7 years ago

A section on "Adding New DB Adaptors" is probably warranted too. Define the interface and options that are passed and the expectations from the combinations from an enduser perspective

👍

In the readme or separate md file?

mxstbr commented 7 years ago

Will write those docs once this PR is done, code-wise 😊

relekang commented 7 years ago

I think this is good now :v:

@mxstbr I ran into a circular reference so I moved repeated char helper to db.js, not sure how to deal with that. Anyhow we can maybe iterate on that later 🙂

relekang commented 7 years ago

Fixed the comments ✌️

mxstbr commented 7 years ago

Merging this and continuing work with subsequent PRs!