Skip to content

Conversation

@nik9000
Copy link
Member

@nik9000 nik9000 commented Apr 13, 2016

No description provided.

@nik9000 nik9000 added >test Issues or PRs that are addressing/adding tests review :Distributed Coordination/Network Http and internode communication implementations v5.0.0-alpha2 labels Apr 13, 2016
@nik9000
Copy link
Member Author

nik9000 commented Apr 13, 2016

@danielmitterdorfer I was trying to debug a non-reproducable failure I had on this test locally (no node available while doing after test cleanup) and I found some funky stuff going on with the generics. Would you mind having a look?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the better option is to add the '@SafeVarargsannotation toNettyHttpClient#post()(which requires the method to befinalbut I'd prefer to make even the classfinal` as it is not intended for inheritance).

@danielmitterdorfer
Copy link
Member

@nik9000 I wonder why the problem appeared in the first place. Do you use Eclipse? I did not have any problem in IDEA and on the command line with Oracle JDK 1.8.0_74-b02.

I left one comment, otherwise LGTM.

Can you please backport this to 2.x too as I just pushed the backport of #17133 where this change comes from?

@danielmitterdorfer danielmitterdorfer removed their assignment Apr 14, 2016
@nik9000
Copy link
Member Author

nik9000 commented Apr 14, 2016

I wonder why the problem appeared in the first place. Do you use Eclipse?

Yes I do, but Eclipse didn't catch it. I add type parameters to things that are missing them when I touch code out of habit and that caused a chain reaction. I suspect the that I hit is something you've already fixed by raising the request buffer but it never reproduced locally for me.

@nik9000 nik9000 force-pushed the request_limit_test branch from 2e782ea to cbb9858 Compare April 14, 2016 11:49
@nik9000 nik9000 force-pushed the request_limit_test branch from cbb9858 to 348ad7a Compare April 14, 2016 14:46
@nik9000 nik9000 merged commit 348ad7a into elastic:master Apr 14, 2016
@nik9000
Copy link
Member Author

nik9000 commented Apr 14, 2016

2.x: 9486bb0 08bd55b

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

Labels

:Distributed Coordination/Network Http and internode communication implementations >test Issues or PRs that are addressing/adding tests v2.4.0 v5.0.0-alpha2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants