Skip to content

Conversation

@Romanx
Copy link
Contributor

@Romanx Romanx commented Feb 12, 2016

See #1810

Not ready for merging and requires feedback. There's one skipped test which I wasn't sure how to do using your test framework,

If you could take a look and provide feedback if their is a way. If not it might be worth investing in making a way as it's a slightly more advanced use case than the other tests I found, hopefully the test explains itself well enough, otherwise just ask.

Test can be found in this commit 8d1906a

@Romanx Romanx changed the title Feature sticky connection Sticky Connection Pool Feb 12, 2016
@Mpdreamz
Copy link
Member

Hi @Romanx we are all travelling and attending elasticon the coming days so we might take a tad longer then usual to review this PR.

Can't wait to dig in though 👍

@Mpdreamz Mpdreamz merged commit 8abf371 into elastic:master Feb 19, 2016
Mpdreamz added a commit that referenced this pull request Feb 19, 2016
@Mpdreamz
Copy link
Member

Thank you for taking the challenge @Romanx 👍 merged this in today 😄

@Romanx
Copy link
Contributor Author

Romanx commented Feb 22, 2016

@Mpdreamz Thanks for letting me have a stab, hopefully it all makes sense. Sorry about the formatting of the commit, didn't realise I didn't have the editor config plugin installed!

@Romanx
Copy link
Contributor Author

Romanx commented Feb 25, 2016

@Mpdreamz did you get to look at the skipped test? I struggled with finding a way to do what I wanted to test with the current test framework, if there's a way i'd be willing to fix up that test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants