Swap out then-redis for another redis lib

[ember-cli-deploy]

Swap out then-redis for another redis lib

[akshayrawat]

Also it would be nice to use a redis library which accept the database in the URL.

then-redis ignores the DB in URL and also ignores the database in options when the URL is used.

[achambers]

@akshayrawat Thanks mate....Will keep this in mind. Fancy flicking us a PR for the change?

[acorncom]

Looks like then-redis was updated recently and supports specifying the DB in the URL, but there may be some bugs there. From what I'm reading here (https://github.com/mjackson/then-redis/blob/57609ba1c6cedbef5686ddc0ba9ec9e58c7989ba/modules/Client.js#L83-L91) looks like database is being pulled from the username slot in a redis://username:password@host/<db> url

[acorncom]

then-redis ignores the DB in URL and also ignores the database in options when the URL is used.

Can confirm this statement, just saw that in action as well ;-)

[acorncom]

An update here. Currently then-redis supports the following Redis config:

redis://db:password@host:port

Per https://www.iana.org/assignments/uri-schemes/prov/redis the config standard should be:

redis://user:secret@localhost:6379/0?foo=bar&qux=baz

[blimmer]

FWIW I switched from then-redis over to ioredis in node-ember-cli-deploy-redis and have been very happy with it. The PR where I made the change is available here.

[ghedamat]

thanks for the suggestion @blimmer!

I'll try to write a CTA to see if anyone is interested in tackling the issue

[blimmer]

I made good progress on this this afternoon, but there's still some tests to be fixed up.

https://github.com/ember-cli-deploy/ember-cli-deploy-redis/compare/master...blimmer:feature/replace-then-redis?expand=1

The previous test implemented stubbed out the Redis client so much that it didn't expose any problems when switching over in #76 . So, the remaining work in this branch is to use the mock, in-memory redis to create higher-fidelity tests.

All tests marked with .skip are remaining in the link above. I'll try to tackle more of them this week, but help is more than welcome 😄

[ghedamat]

first, thanks for this @blimmer ! this issue has been neglected for too long, great to see some momentum!

second I think you're definitely right and the previous mocking was way too aggressive, we need to ensure that what we let happen in #76 is not gonna happen again.

I'll definitely see if I can make some time in the near future to look more at the tests but. at the minimum feel free to ping me on slack (@ghedamat) if you get stuck on something.

I'm also sure @achambers and @lukemelia will be around there to help if needed!

thanks again!

[blimmer]

I got down to 8 pending tests today and tested this with my app with yarn link. I should be able to get this over the line tomorrow.

[blimmer]

See #86