Skip to content

Loading…

Add support for routing parameter to search requests. #258

Closed
wants to merge 1 commit into from

3 participants

@dylanahsmith

Addresses specifying the routing parameter for search. See issue #88.

@karmi
Owner

Hi Dylan, I don't know, this seems to me as another case of trying to hack some ES options into Tire methods. I've said it previously, we need to expose all the options, namely search_type etc somehow. I'm reluctant to do it in piecemeal fashion.

@dylanahsmith

we need to expose all the options, namely search_type

The difference is that those are body parameters, but this one is a URL parameter and should be a place that is easier to share with the Tire.search(indices, payload) code path of Tire.search. See pull request #259 for how I made this usable for both code paths of Tire.search.

this seems to me as another case of trying to hack some ES options into Tire methods

I don't see how not implementing unrelated functionality somehow makes this code a hack. It certainly doesn't make it harder to implement support for search body parameters.

I also implemented this in a method that allows that makes it easier to add additional URL parameters (although the only other I can find is pretty) by adding them to the slice method, and it is implemented in a way that allows a methods in Tire::Search::Search to set a query parameter by modifying @params.

If you want, it would be very easy to modify this code to only expose the routing parameter through a method in Tire::Search::Search. But so far you haven't expressed any opinion either way in issue #88, so I thought I would try to get some feedback.

Edit: Looked at search_type more carefully, and it does appear to be a url parameter as well, even though it was listed along with body parameters in the docs.

@erickt

@dylanahsmith: search_type is only a url parameter, it cannot be passed in the body.

@karmi: This approach seems fine to me. I'd be fine on punting on #198 for this.

@karmi karmi added a commit that closed this pull request
@karmi Added support for passing search params (`search_type`, `timeout`, et…
…c.) to search requests

Previously, passing of parameters such as search_type [http://www.elasticsearch.org/guide/reference/api/search/search-type.html]
to search requests wasn't supported, so in cases like getting counts or being interested in just facets,
the workaround was to use `size 0`.

This commit implements passing of parameters to search requests as URL-encoded string.

Just pass the parameters you need to the `search` method:

    s = Tire.search 'articles-test', :search_type => 'count' do
      query { term :tags, 'ruby' }
    end

    p s.results.total
    # => 2
    p s.json['hits']
    # => {"total"=>2, "max_score"=>0.0, "hits"=>[]}

See the possible list of request parameters here:

* http://www.elasticsearch.org/guide/reference/api/search/request-body.html
* http://www.elasticsearch.org/guide/reference/api/search/uri-request.html

Closes #242, closes #198, closes #196, closes #89, closes #88, closes #258, closes #100.
d764c0d
@karmi karmi closed this in d764c0d
@nicolas2bonfils nicolas2bonfils referenced this pull request
Commit has since been removed from the repository and is no longer available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 29, 2012
  1. @dylanahsmith
This page is out of date. Refresh to see the latest.
Showing with 16 additions and 2 deletions.
  1. +8 −2 lib/tire/search.rb
  2. +8 −0 test/unit/search_test.rb
View
10 lib/tire/search.rb
@@ -12,6 +12,7 @@ def initialize(indices=nil, options = {}, &block)
@options = options
@path = ['/', @indices.join(','), @types.join(','), '_search'].compact.join('/').squeeze('/')
+ @params = options.slice(:routing)
block.arity < 1 ? instance_eval(&block) : block.call(self) if block_given?
end
@@ -25,7 +26,9 @@ def response
end
def url
- Configuration.url + @path
+ query = @params.to_param
+ query = "?" + query unless query.empty?
+ Configuration.url + @path + query
end
def query(&block)
@@ -91,7 +94,10 @@ def perform
end
def to_curl
- %Q|curl -X GET "#{self.url}?pretty=true" -d '#{self.to_json}'|
+ @params[:pretty] = true
+ curl = %Q|curl -X GET "#{self.url}" -d '#{self.to_json}'|
+ @params.delete(:pretty)
+ curl
end
def to_hash
View
8 test/unit/search_test.rb
@@ -35,6 +35,14 @@ class SearchTest < Test::Unit::TestCase
assert_match %r|index/bar/_search|, s.url
end
+ should "allow specify routing query parameter" do
+ s = Search::Search.new('index', :routing => 123) do
+ query { string 'foo' }
+ end
+
+ assert_match %r|index/_search\?routing=123|, s.url
+ end
+
should "allow to pass block to query" do
Search::Query.any_instance.expects(:instance_eval)
Something went wrong with that request. Please try again.