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

Wait for the future response #899

Merged
merged 2 commits into from
Jun 17, 2019
Merged

Wait for the future response #899

merged 2 commits into from
Jun 17, 2019

Conversation

marcvdm
Copy link
Contributor

@marcvdm marcvdm commented Jun 14, 2019

The current SniffingConnectionPool uses the sniff method on the Connection class but this needs to return an array. The current implementation fails because a future request is returned.

This merge requests adds the wait function for the return.

@ezimuel
Copy link
Contributor

ezimuel commented Jun 14, 2019

The Connection::performRequest() returns a mixed type. When I did the code refactoring, introducing type hints and return types, I struggled between the old implementation and the new one.
Maybe a better idea is to remove the return type on the sniff() function, in order to preserve asynchronous feature. What do you think? Thanks for reporting this issue.

@ezimuel ezimuel self-assigned this Jun 14, 2019
@marcvdm
Copy link
Contributor Author

marcvdm commented Jun 14, 2019

I'v updated the pull request and remove the typehints. This way the response can be an array or FutureArray and the parseClusterState wil iterate over an array or ArrayAccess variable

@ezimuel ezimuel added this to the 7.0.1 milestone Jun 14, 2019
@ezimuel ezimuel merged commit 1de4044 into elastic:master Jun 17, 2019
@ezimuel
Copy link
Contributor

ezimuel commented Jun 17, 2019

Thanks @marcvdm

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.

2 participants