Update Redis and libraries

Ma27 / rediscala

Non-blocking, Reactive Redis driver for Scala (with Sentinel support)

Apache License 2.0

12 stars 8 forks source link

Update Redis and libraries #14

Closed kardapoltsev closed 5 years ago

kardapoltsev commented 5 years ago

@Ma27 WDYT?

Ma27 commented 5 years ago

First of all sorry for the delay (again)! I'm currently having the following issues with this PR (but I can fix them myself):

I'd like to remove the ByteString assertions entirely. We already experienced behavioral differences between several platforms, so this should be dropped entirely
~I need to find a better solution for the redis ruby script. The approach with the global gem installation never worked for me, I'm currently thinking about using i.e. bundler (or switch back to a Nix(OS) based approach).~ EDIT: just seen that thsi isn't even needed anymore which is even better, thanks!

Let's see how farI get, but I plan to merge this during the next days :)

Ma27 commented 5 years ago

The current problem is that there are some heavy tests that fial randomly. I experienced this before on GitLab CI and other environments. As this behavior is now observable on Travis CI as well, there's yet another reason to rework the test suite.

Ma27 commented 5 years ago

@kardapoltsev have you tested the test suite against Travis multiple times or just once?

As random tests with heavy IO fail, I doubt that it's related to my commits from last lights to be honest (I've experienced it before, so I doubt that this PR is related). May I ask if you have an idea how to fix it? Otherwise I'd have a more detailed look at the test suite to hopefully find a fix next week.

kardapoltsev commented 5 years ago

Yes, I have this issue with tests before PR and in RP too.

-- Regards, Alexey

On Wed, Feb 20, 2019, 6:01 AM Maximilian Bosch <notifications@github.com wrote:

@kardapoltsev https://github.com/kardapoltsev have you tested the test suite against Travis multiple times or just once?

As random tests with heavy IO fail, I doubt that it's related to my commits from last lights to be honest (I've experienced it before, so I doubt that this PR is related). May I ask if you have an idea how to fix it? Otherwise I'd have a more detailed look at the test suite to hopefully find a fix next week.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Ma27/rediscala/pull/14#issuecomment-465347200, or mute the thread https://github.com/notifications/unsubscribe-auth/ABEFSmRDPFF0-EQAHi-ynCZdd3cxyFC7ks5vPIIsgaJpZM4ZzXa_ .

Ma27 commented 5 years ago

Hmm I see. If it's okay for you I'd merge this for now as getting rid of redis-trib.rb is IMHO a great step forward in our test suite. To get those tests fixed I guess that we need to improve our tests that do heavy IO.

kardapoltsev commented 5 years ago

-- Regards, Alexey

On Wed, Feb 20, 2019, 6:14 AM Maximilian Bosch <notifications@github.com wrote:

Hmm I see. If it's okay for you I'd merge this for now as getting rid of redis-trib.rb is IMHO a great step forward in our test suite. To get those tests fixed I guess that we need to improve our tests that do heavy IO.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Ma27/rediscala/pull/14#issuecomment-465351015, or mute the thread https://github.com/notifications/unsubscribe-auth/ABEFSlJfKFDPtm-6KC4Dh2mtfamB7jllks5vPIVZgaJpZM4ZzXa_ .

Ma27 commented 5 years ago

@kardapoltsev thanks a lot! Unless anybody else is faster, I'll have a deeper look at the tests next month :)