search

twitter / storehaus

Storehaus is a library that makes it easy to work with asynchronous key value stores
Apache License 2.0
465 stars 92 forks source link

Cleanup storehaus-mysql #327

Closed BenFradet closed 8 years ago

BenFradet commented 8 years ago

@johnynek could you trigger a new build when you get the chance, twitter's mvn is having issues again. Nevermind the failing build was the one before the rebase.

BenFradet commented 8 years ago

Thanks for your review, I think I've taken care of everything you mentioned.

I'd still like to know why prefer Iterator.continually over Stream.continually.

johnynek commented 8 years ago

This may be superstition but when I know I will consume the collection, I convert to Iterator and do the operation on that. The memorization of Stream I assume to be not free.

What said, I should benchmark it.

PS: I'm on vacation this week so maybe someone else can merge or I can get to it when I get back. On Tue, Aug 30, 2016 at 06:27 Ben Fradet notifications@github.com wrote:

Thanks for your review, I think I've taken care of everything you mentioned.

I'd still like to know why prefer Iterator.continually over Stream.continually.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/twitter/storehaus/pull/327#issuecomment-243497506, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEJdsGgQTpZqQA3YhKqzrJGGadA86wHks5qlFnfgaJpZM4JurMH .

BenFradet commented 8 years ago

Ok no worries I'll be waiting till next week. Have a great holiday!

BenFradet commented 8 years ago

Still investigating why the tests are failing on multiGet/multiPut.

johnynek commented 8 years ago

@BenFradet do they fail locally as well?

BenFradet commented 8 years ago

yup, apparently it doesn't come from the injection since I haven't seen any failure from toTwitterFuture.

BenFradet commented 8 years ago

@johnynek should be fixed, I'll comment on the problems inline :+1: