bugfix: setup routes (and their redis client) on demand only

[juckerf]

I encountered the problem, that if I have a non-default redis server (alternate port, ip, ...), feathers kept crashing because when requiring feathers-hooks-rediscache routes/cache.js is executed immediately (and it tries to create a redis connection on the default port, ip, ...). So I put the setup of the routes in a function routes. The function returns the router so it can still be used as callback for feathers' app.use(). Probably not the most elegant solution but it works (unfortunately with a change in the usage of the module).

[coveralls]

Coverage decreased (-4.1%) to 73.936% when pulling 2925d0eb5fb6bb43132fabb9c65634ecb806f2d3 on juckerf:master into f0c2a4d5530856baa64b63cb44aeae91b9eb147f on idealley:master.

[coveralls]

Coverage decreased (-4.1%) to 73.936% when pulling 1482ae906eed6d593b2e7cbf7cd220731073e7a1 on juckerf:master into f0c2a4d5530856baa64b63cb44aeae91b9eb147f on idealley:master.

[idealley]

One question. We call twice the redis.createClient(), once in the redisClient.js file and the other time in the routes/cache.js. The first time it is configurable and the second time it uses defaults...

Now you have wrapped the routes in a function. I guess we could pass app as a parameter, and use the configured client available under 'app.redisClient'. One instantiation, and configuration is added to the routes. What do you think?

[juckerf]

it would indeed be the best solution if we have to call redis.createClient only once.

hm.. I wasn't thinking about simply passing app to the function (I just wanted to get feathers running again :D). sounds like a reasonable solution. On 27/09/2017, 22:52 Samuel Pouyt notifications@github.com wrote: One question. We call twice the redis.createClient(), once in the redisClient.js file and the other time in the routes/cache.js. The first time it is configurable and the second time it uses defaults... Now you have wrapped the routes in a function. I guess we could pass app as a parameter, and use the configured client available under 'app.redisClient'. One instantiation, and configuration is added to the routes. What do you think?

—You are receiving this because you authored the thread.Reply to this email directly, view it on GitHub, or mute the thread.

{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/idealley/feathers-hooks-rediscache","title":"idealley/feathers-hooks-rediscache","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/idealley/feathers-hooks-rediscache"}},"updates":{"snippets":[{"icon":"PERSON","message":"@idealley in #16: One question. We call twice the redis.createClient(), once in the redisClient.js file and the other time in the routes/cache.js. The first time it is configurable and the second time it uses defaults...\r\n\r\nNow you have wrapped the routes in a function. I guess we could pass app as a parameter, and use the configured client available under 'app.redisClient'. One instantiation, and configuration is added to the routes. What do you think?"}],"action":{"name":"View Pull Request","url":"https://github.com/idealley/feathers-hooks-rediscache/pull/16#issuecomment-332651734"}}}

[idealley]

Ok I have tested it.

  function routes(app) {
  const router = express.Router();
  const client = app.get('redisClient');
  const h = new RedisCache(client);
  ...

Then we configure it like this: app.use('/cache', routes(app));

I think this makes the routes configurable and answer the partially solved issue #2 with your commit d267c76

I will add it and merge.

[idealley]

I am also bumping the version to 0.3.x as the API changes. Thank you for the nice idea.