Replace then-redis with ioredis #86
Conversation
Also, significantly reduce mocking in the test suite by using an in-memory redis client. Fixes ember-cli-deploy#51 Fixes ember-cli-deploy#74
blimmer
force-pushed
the
feature/replace-then-redis
branch
from
390363d
to
9b03b61
Compare
| @@ -6,7 +6,7 @@ module.exports = CoreObject.extend({ | |||
| init: function(options, lib) { | |||
| this._super(); | |||
| var redisOptions = {}; | |||
| var redisLib = lib; | |||
| var RedisLib = lib; | |||
There was a problem hiding this comment.
I capitalized this because you call the the constructor with ioredis instead of createClient. It made more sense to me to call new RedisLib() instead of new redisLib().
| @@ -21,15 +21,15 @@ module.exports = CoreObject.extend({ | |||
| } | |||
|
|
|||
| if (options.database) { | |||
| redisOptions.database = options.database; | |||
| redisOptions.db = options.database; | |||
There was a problem hiding this comment.
I left the API the same here, but then-redis was pretty much the only library to call this parameter database instead of db.
| } | ||
| }); | ||
|
|
||
| return RSVP.all(promises); |
There was a problem hiding this comment.
I was seeing flaky tests when we weren't waiting on these promises, so I wrapped it in an RSVP.all so we wait for all the redis operations to complete.
| - '4' | ||
| - '6' | ||
| - '8' | ||
| - '10' |
There was a problem hiding this comment.
I've added node 8 and 10 since they're technically supported.
https://github.com/ember-cli/ember-cli/blob/master/docs/node-support.md
|
No problem - thank you for reviewing. This should be safe to merge and release. |
|
@blimmer I discussed to day w/ @lukemelia I'm gonna do a light QA of this pr on one of our apps at work just as a second sanity check and then I'll merge and release! Will try to get to this during the weekend! If I don't please nag me here or on slack! thanks again ! |
What Changed & Why
This PR replaces then-redis with ioredis. then-redis has been deprecated, so we needed to replace it with something.
I chose ioredis over the
node_redispackage because they require Node 8 or higher for native promise support, or you have to promisify via Bluebird.Also note that I've used ioredis < 4 because of their requirement of Node 6 or greater. As ember-cli support drops out for Node 4, we should be able to upgrade to the newer versions.
I've tested this using
yarn linkdeploying my Ember 2.16 application and have not had any problems.Related issues
Fixes #51
Fixes #74
PR Checklist
People
@ghedamat