search

bbc / alephant-broker

Brokers requests for alephant components
MIT License
4 stars 3 forks source link

Remove dalli-elasticache wrapper #82

Closed woodyblah closed 5 years ago

woodyblah commented 5 years ago

The dalli-elasticache wrapper is for auto-discovering elasticache endpoints. However due to recent updates with AWS Elasticache, this behaviour is no longer needed. What's more is this gem has a non parameterised 60s timeout when autodiscovering and causes Alephant to hang for a long time on each request if the provided elasticache endpoint is unreachable.

This change switches to using the Dalli client directly.

woodyblah commented 5 years ago

I've also had to pin the travis version of JRuby back, as it appears to be failing on 9.2.0.0

I'll create a ticket for upgrading/fixing ruby 2.5 and 9.2.0.0 support

samfrench commented 5 years ago

Any specs for the issue? We only know it's fixed due to debugging...

woodyblah commented 5 years ago

It's difficult to write specs for the specific issue as the bug lies somewhere inside the dalli-elasticache gem, not within our code, and also only seems to apply when autodiscovering an elasticache endpoint which we wont want it to actually do in our tests (or at all anymore). I'll add some generic specs for this client class though as it looks completely untested(❗️) , but even if this was tested I imagine the dalli-elasticache behaviour would have been mocked out anyway.