search

hapijs / catbox-redis

Redis adapter for catbox
Other
69 stars 63 forks source link

chore(deps): update stuff #89

Closed SimonSchick closed 6 years ago

SimonSchick commented 6 years ago

This allows us to use ioredis v4 with this package, there should be no compatibility issues. I also illiminated hoek as a dependency, not really needed given there is a native assert and added 2 (technically) async functions as async to make sure they always reject and never throw sync.

SimonSchick commented 6 years ago

@marcuspoehls

SimonSchick commented 6 years ago

Minor feature creep, lint annoyed me so I converted it to a class, please review without whitespace changes.

marcuspoehls commented 6 years ago

@SimonSchick Hey Simon, I’m seeing your updates on this PR. Thank you for keeping it up to date. I ran some tests and implementations in a separate PR.

You removed the sentinel test. Is that intentionally? I know this is hard to test locally or with Travis. That’s why I added a mock server based on the code from ioredis. My idea is to use the mock server for cluster testing as well.

https://github.com/hapijs/catbox-redis/pull/86

I know we’ve discussed all the testing around ioredis in the related issue #88. I still think we should make sure that the package works properly even though it mostly passes through options and params to ioredis. That’s why I added the mock server for testing.

What do you think about the mock server idea?

SimonSchick commented 6 years ago

The removal of sentinel tests was not intended, my bad.

As discussed in another PR I don't see any value in continuing to add more pass-through functionality to ioredis, instead the client should be passed directly and about 1/3 of the current code could be purged.

Internally I'm no longer using this package and refactored and vendored catbox redis https://gist.github.com/SimonSchick/ff7d709de5c995bdf8374e5d4bb553aa

SimonSchick commented 6 years ago

Sorry, I currently don't have time to finish this. Anyone else is free to use the code I wrote and create their own PW.

marcuspoehls commented 6 years ago

@SimonSchick Thank you for your work Simon!

lock[bot] commented 4 years ago

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.