feathersjs-ecosystem / feathers-sync

Synchronize service events between Feathers application instances
MIT License
222 stars 41 forks source link

redis adapter #2

Closed kc-dot-io closed 8 years ago

kc-dot-io commented 8 years ago

Summary

I want to be able to use feathers-sync with redis to distribute events across nodes.

Changes

kc-dot-io commented 8 years ago
daffl commented 8 years ago

This is awesome, thank you! It looks like Travis CI does not have a Redis server running. Adding

services:
  - redis-server

To .travis.yml should fix it. Once the tests are passing I will merge and after that do a little bit of refactoring I've been planning on for a while already (e.g. all our up-to-date plugins have been migrated to ES6).

kc-dot-io commented 8 years ago

Cool, are you happy with it the way it is based on the notes I added above? I'm happy to leave those be if you have some bigger refactors you want to throw down.

I'll add the redis-server now.

kc-dot-io commented 8 years ago

I'm adding tests for redis - don't merge yet

daffl commented 8 years ago

Nice, the tests passed. I'd say let's merge it, I'll migrate everything to ES6 and then we can iterate to see if there are ways to improve it. Sounds good?

daffl commented 8 years ago

Well once you added the tests I mean.

kc-dot-io commented 8 years ago

Yeah I'm just fixing a couple things real quick on test coverage. I'll ping you shortly.

kc-dot-io commented 8 years ago

@daffl ok i'm done! merge away.

kc-dot-io commented 8 years ago

btw, it should be ready to be npm published to feathers-sync now if you want to release it there

marshallswain commented 8 years ago

This is so awesome! Thanks for adding this.

daffl commented 8 years ago

Perfect. I'll update it to ES6 later today. One thing I may add to the refactoring is removing the redis and mubsub dependencies and passing them in your application. Something like:

var sync = require('feathers-sync');
var redis = require('redis');

app.configure(sync(sync.redis(redis)));

That way this module doesn't install dependencies you don't use (we've done a similar thing for the database adapters). Thoughts?

ekryski commented 8 years ago

Awe yeeeaaaah! Thanks @slajax!

kc-dot-io commented 8 years ago

@daffl I agree with your suggestions. I was thinking exporting them directly would make sense initially because they probably have different options arguments but also it would be an even more ideal scenario to abstract out the dependencies to other repos to save installing mubpub if you use redis so :+1:

Will the current version be published to NPM as a 0.2.0 or do you intend to do these refactors first? My reason for asking is purely selfish, I want to use the current version in my builds up until that refactor is ready.