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

Fix scroll action and next on searchResult #172

Merged
merged 5 commits into from
Jan 19, 2017

Conversation

dbengsch
Copy link
Contributor

No description provided.

@dbengsch dbengsch added the bug label Jan 18, 2017
@dbengsch dbengsch self-assigned this Jan 18, 2017
@@ -511,6 +511,10 @@ Collection.prototype.scroll = function (scrollId, options, filters, cb) {
options = {};
}

if (!options.scroll) {
throw new Error('Collection.scroll: scroll is required');
Copy link
Contributor

Choose a reason for hiding this comment

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

scroll parameter is not required to do scroll request (that is only to force refreshing scroll id)

Copy link

@ballinette ballinette Jan 19, 2017

Choose a reason for hiding this comment

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

Hmmm are your sure ? the ES documentation gives always a scroll parameter in its examples:
https://www.elastic.co/guide/en/elasticsearch/reference/5.0/search-request-scroll.html

POST  /_search/scroll 
{
    "scroll" : "1m", 
    "scroll_id" : "DXF1ZXJ5QW5kRmV0Y2gBAAAAAAAAAD4WYm9laVYtZndUQlNsdDcwakFMNjU1QQ==" 
}

(but I did not test it without the parameter, so maybe their documentation is not exactly good)

Copy link
Contributor

Choose a reason for hiding this comment

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

As discussed together, LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants