Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add unlimited method to fetch all records from ES #393

Merged
merged 1 commit into from
May 27, 2016

Conversation

sergey-kintsel
Copy link
Contributor

It removes offset and sets response limit to the total_count value

@sergey-kintsel sergey-kintsel force-pushed the add-fetch-all branch 2 times, most recently from 61c12c2 to aa6af74 Compare May 27, 2016 08:21
@@ -5,7 +5,7 @@ class Query
class Criteria
include Compose
ARRAY_STORAGES = [:queries, :filters, :post_filters, :sort, :fields, :types, :scores]
HASH_STORAGES = [:options, :request_options, :facets, :aggregations, :suggest, :script_fields]
HASH_STORAGES = [:options, :request_options, :facets, :aggregations, :suggest, :script_fields, :search_options]
Copy link
Contributor

Choose a reason for hiding this comment

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

move it before request_options please

@sergey-kintsel sergey-kintsel force-pushed the add-fetch-all branch 2 times, most recently from e41928e to 8a9a6bb Compare May 27, 2016 09:53
@@ -96,13 +96,15 @@
specify { expect(subject.limit(10)).not_to eq(subject) }
specify { expect(subject.limit(10).criteria.request_options).to include(size: 10) }
specify { expect { subject.limit(10) }.not_to change { subject.criteria.request_options } }
specify { expect(subject.offset { 20/2 }.criteria.request_body[:body]).to include(size: 10) }
Copy link
Contributor

Choose a reason for hiding this comment

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

This shell not pass!

@pyromaniac pyromaniac merged commit ec5ed4c into toptal:master May 27, 2016
@pyromaniac
Copy link
Contributor

Awesome! Thanks!

@sergey-kintsel sergey-kintsel deleted the add-fetch-all branch July 25, 2016 09:28
@binarydev
Copy link

@sergey-kintsel does this require ES 5.x or a special max_results_window value? Tried to use it locally with my ES 2.3 installation, and it complains about going beyond the max results window

@sergey-kintsel
Copy link
Contributor Author

@binarydev Hi! It does not require the latest ES, but it does require max_results_window. The problem is that most likely you are trying to load too much data. This method is a bit unsafe I would say. So, if you really need that much data - change max results window

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.

4 participants