Skip to content

Conversation

Mpdreamz
Copy link
Member

@Mpdreamz Mpdreamz commented Dec 7, 2016

Now that 5.x no longer returns http_address we need to give precedence
to publish_address over bound_address

Copy link
Contributor

@gmarz gmarz left a comment

Choose a reason for hiding this comment

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

LGTM 👍. As a future change, it'd be really nice if we could write some integration tests for our connection pools.

@@ -5,13 +5,29 @@

namespace Elasticsearch.Net
{
public class SniffResponse
public static class SniffParser
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@Mpdreamz
Copy link
Member Author

Mpdreamz commented Dec 8, 2016

In the 2.x branch we look at http_address

which will yield "http_address" : "elastic.co/35.160.254.14:9200",

and in 2.x the http section of the response returns the same value for http_address:

"http" : {
        "bound_address" : [ "127.0.0.1:9200", "[::1]:9200" ],
        "publish_address" : "elastic.co/35.160.254.14:9200",
        "max_content_length_in_bytes" : 104857600
}

Which is why this does not need to be ported to 2.x as we do the right thing there already

Now that 5.x no longer returns http_address we need to give precedence
to `publish_address` over `bound_address`
@Mpdreamz Mpdreamz merged commit addc386 into master Dec 8, 2016
Mpdreamz added a commit that referenced this pull request Dec 8, 2016
Now that 5.x no longer returns http_address we need to give precedence
to `publish_address` over `bound_address`
@Mpdreamz
Copy link
Member Author

Mpdreamz commented Dec 8, 2016

ported to 5.x

@Mpdreamz Mpdreamz deleted the fix/publish_host branch December 8, 2016 10:08
@russcam russcam added the v5.0.0 label Dec 13, 2016
awelburn pushed a commit to Artesian/elasticsearch-net that referenced this pull request Nov 6, 2017
Now that 5.x no longer returns http_address we need to give precedence
to `publish_address` over `bound_address`
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.

3 participants