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

Converting indexPattern to indexList is now async #5173

Merged
merged 2 commits into from
Oct 22, 2015

Conversation

epixa
Copy link
Contributor

@epixa epixa commented Oct 22, 2015

Since there will soon be a pre-flight request to elasticsearch on
certain index patterns when trying to determine which indices match, the
conversion must now happen asynchronously. The function now returns a
promise that is fulfilled with the matching array of index names.

This is necessary for #4342, but it does not complete that ticket.

Since there will soon be a pre-flight request to elasticsearch on
certain index patterns when trying to determine which indices match, the
conversion must now happen asynchronously. The function now returns a
promise that is fulfilled with the matching array of index names.
@@ -100,7 +107,7 @@ define(function (require) {
};

SegmentedReq.prototype.isIncomplete = function () {
return this._queue.length > 0;
return !this._queueCreated || this._queue.length > 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

might read better as this._queueCreated &&. I missed the ! at first and thought this was backwards

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That wouldn't be the same conditional logic though: this is only negating the first half of the condition.

@spalger
Copy link
Contributor

spalger commented Oct 22, 2015

Seems to work great, just waiting for responses to my comments. Otherwise LGTM

These changes are internally cosmetic for the sake of adding clarity for
developers and ensuring that state is changed in the most appropriate
places to help prevent future regressions.
@epixa
Copy link
Contributor Author

epixa commented Oct 22, 2015

I pushed two changes here:

  • Moved _createQueue = true to a more appropriate place
  • Broke up complex conditional logic into more expressive variables to add clarity

@spalger
Copy link
Contributor

spalger commented Oct 22, 2015

LGTM

spalger added a commit that referenced this pull request Oct 22, 2015
Converting indexPattern to indexList is now async
@spalger spalger merged commit ab2c6d6 into elastic:master Oct 22, 2015
@epixa epixa deleted the 4342-async-indexlist branch October 27, 2015 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants