ES 1.x response filtering

[grantr]

ES 1.x added response filtering to the path for cluster state and node stats:
http://www.elasticsearch.org/guide/en/elasticsearch/reference/current/cluster-nodes-stats.html#cluster-nodes-stats
http://www.elasticsearch.org/guide/en/elasticsearch/reference/current/cluster-state.html#cluster-state

We now support this with parameters on the cluster.state and nodes.stats methods.

client.cluster.state(:metrics => 'metadata', :indices => 'test*')
client.nodes.stats(:stats => 'os')

/cc @TwP

[TwP]

Cool - thanks for updating these methods. This is good to ship. 👍

I'm wondering if a better way to handle these incompatibilities would be to make the 1.0 versions the default and provide a 0.90 compatibility module. When the client first connects, it checks the ES version and then includes the 0.90 compatibility module. This module would replace methods with 0.90 versions.

It would make the code a bit cleaner - no version checks scattered throughout the codebase. That's the only advantage that I can see. Eventually we can scrap the 0.90 support all together. When we hit the point we just delete the compatibility module and the one version check where it gets included.

Thoughts on that approach?

[grantr]

I think that's a good pattern, not only for 0.90 -> 1.x (which we're mostly done with) but for future incompatibilities as well.

We do lose the ability to do very granular branching on versions. No more if 1.x this else that. That could lead to copied methods and maintenance issues.

I've never had to do this kind of version-compatibility maintenance over a long period, but I can think of a few people who might have. @brianmario and @jnunemaker, given your experience maintaining client libraries, would you put version conditionals inside methods or use version-specific mixins?

[jnunemaker]

Typically clients talk to one or the other. What I've seen (not sure that I've done this) is having a different require for different versions and making one the default.

require "elastomer" # defaults to 1.0

# or require it
require "elastomer/1.0"

# or require 0.9
require "elastomer/0.9"

# i've also seen env var 
ENV["ELASTIC_SEARCH_VERSION"] = "1.0"
require "elastomer" # requires 1.0

It seems rare to talk to both, but I don't know your use case. If that is common, then doing something like this makes sense where each explicit code path forks. In general I am not a fan of making a network call for stuff like getting a version. While it makes the code easier, most people know the version they are on and could supply it to the client (and it could be defaulted). I HATE network calls. :) I could see allow to supply the version to avoid the info network call hit even as the code is now.

[jnunemaker]

I would be inclined to do if client.path_segment_state_filtering? or something to tie that logic up in the client.

[TwP]

My vote here is to merge in this PR and then open a separate PR to revisit the version-specific methods and how we handle them. There are a few different approaches we can take. It would behoove us to do some research.

[grantr]

Oops forgot about this one.

@jnunemaker in this case a single ruby process will talk to multiple versions of ES, and app code often is not aware of which version is used, so the version needs to be detected at runtime.

I'll go ahead and merge, then open a new issue about versioning.