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 support for db plugins #8

Closed relekang closed 7 years ago

relekang commented 7 years ago

As I suggested on twitter it would be nice to have support for db plugins. I am am more than happy to implement this. There are a few things that needs to be considered. How should the api be? I am thinking the same as db.js is today just with promises for everything, like you mentioned on twitter. Should plugins be in this repo? I think it would be nice if they were external packages e.g. micro-analytics-db-redis.

mxstbr commented 7 years ago

You're definitely thinking in the right direction. I want the default to be flat-file-db due to easy of usage, which shall still live in this repo.

Using a different database should be as easy as running $ micro-analytics --db redis, where "redis" then requires the micro-analytics-adapter-redis package. (I'm thinking all the other packages should live in their own repos of the people who make them, since I might or might not use them and don't want to end up maintaining something I don't know)


The first step towards this is that we have to stop mocking db.js in tests. Instead, we should be mocking flat-file-db, and making db.js be flat-file-adapter.js.


The Adapter Spec:

The methods we use right now are db.get, db.put, db.keys and db.has, all of which should be changed to return promises. I actually think that flat-file-db has something wrong, which is db.keys – it would be much more useful if the db had a method called db.getAll (name tbd), which would automatically return all the data. This would also allow us to move filtering over to the databases, instead of doing it on the returned data. (which is slower)

This means we'd end up with these methods for the adapters:

What do you think?

mxstbr commented 7 years ago

Also, should we future-proof adapters? What other methods could we ever need?

Is there precedence of doing something like this somewhere? What database adapters for which projects exist today?

AriTheElk commented 7 years ago

My first reaction to seeing this repo was "awesome, how can I use firebase as my backend?". Definitely looking forward to the ability to create adapters 👌

The spec you have above looks pretty solid, would easily allow for local and remote storage solutions. Are the before and after filters going to be unix timestamps?

relekang commented 7 years ago

The spec looks good. It might be nice to have subscribe and unsubscribe if we want to support live updates. A bit unsure about that tho, what do you think?

relekang commented 7 years ago

Also do we need a delete call?

juandjara commented 7 years ago

That spec looks solid. You can check #9 for an example of a redis adapter with all methods using promises.

relekang commented 7 years ago

@juandjara I missed that PR, also interested in a redis backend so i started to create an adapter on for it (mostly to think about the API).

btw: I have a wip/poc branch for moving flat file to adapters here https://github.com/mxstbr/micro-analytics/compare/master...relekang:move-flat-file-db-to-adapter. It just need new tests for the filtering.

mxstbr commented 7 years ago

Are the before and after filters going to be unix timestamps?

Yes, I think it's fair to require users to convert their human-readable times to UTC before they send them to us if they need them. (see the discussion in #3)

It might be nice to have subscribe and unsubscribe if we want to support live updates. A bit unsure about that tho, what do you think?

Good call, we should definitely have those! They should probably return an observable based on the new spec? https://github.com/tc39/proposal-observable What do you think?

Also do we need a delete call?

Not sure about this and unsubscribe. Are they really worth having? I don't think you'd ever delete a view or unsubscribe?

relekang commented 7 years ago

Good call, we should definitely have those! They should probably return an observable based on the new spec? https://github.com/tc39/proposal-observable What do you think?

Yes, that sounds like a good idea. I have about zero experience with observables, but from what I understand this seems like a good use case.

Not sure about this and unsubscribe. Are they really worth having? I don't think you'd ever delete a view or unsubscribe?

Agree, just wanted to ask since it was in the current implementation. If we go for observables I don't think we need unsubscribe either.

mxstbr commented 7 years ago

Yes, that sounds like a good idea. I have about zero experience with observables, but from what I understand this seems like a good use case.

The bigger question is how we then implement the actual live updates from the server to the client, but I guess I'll just open another issue for that.