[elasticsearch] Remove hostname matching logic #1657
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
olivielpeau
force-pushed
the
olivielpeau/remove-elasticsearch-hostname-matching
branch
from
fa18ec5 to
e0a3bc4
Compare
| # Check the interface addresses against the primary address | ||
| return primary_addrs in ips | ||
| for metric in stats_metrics: | ||
| desc = stats_metrics[metric] |
There was a problem hiding this comment.
Nitpick but this loop can be simplified: for metric, desc in stats_metrics.iteritems(). This way is also usually faster than iterating and looking up by key
olivielpeau
force-pushed
the
olivielpeau/remove-elasticsearch-hostname-matching
branch
from
e0a3bc4 to
8cbf5ac
Compare
|
Thanks for the review @talwai ! I've changed the loop. |
LeoCavaille
force-pushed
the
olivielpeau/remove-elasticsearch-hostname-matching
branch
from
8cbf5ac to
cc7f3f4
Compare
The hostname matching is not needed anymore as: - since PR #1181 we only ask the _local node for stats when `is_external` is set to `false` - we don't match the hostname at all when `is_external` is `true` Matching hostnames can also filter out legitimate data when the local elasticsearch node reports a different hostname. See also issue #457
It makes more sense overall, cluster_stats must be used when the user targets an external URL (not localhost) and he wants to get stats for a whole cluster, otherwise it will only query for the local node by default.
Test new ES releases, add new branch 1.6.x. Also not run the kill task in the `before_cache` section so that SKIP_CLEANUP still works for instance.
LeoCavaille
force-pushed
the
olivielpeau/remove-elasticsearch-hostname-matching
branch
from
dab560a to
317ccf8
Compare
yannmh
added a commit
that referenced
this pull request
Jun 9, 2015
…h-hostname-matching [elasticsearch] Remove hostname matching logic
|
Changes Unknown when pulling 317ccf8 on olivielpeau/remove-elasticsearch-hostname-matching into ** on master**. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The hostname matching is not needed anymore as:
is_externalis set tofalseis_externalistrueMatching hostnames can also filter out legitimate data when the
local elasticsearch node reports a different hostname.
See also issue #457