Skip to content

Conversation

@javanna
Copy link
Member

@javanna javanna commented Mar 25, 2020

This commit renames wait_for_completion to wait_for_completion_timeout in submit async search and get async search.
Also it renames clean_on_completion to keep_on_completion and turns around its behaviour.

Closes #54069

javanna added 2 commits March 25, 2020 16:36
This commit renames wait_for_completion to wait_for_completion_timeout in submit async search and get async search.
Also it renames clean_on_completion to keep_on_completion and turns around its behaviour.

Closes elastic#54069
@javanna javanna added >enhancement :Search/Search Search-related issues that do not fall into other categories v8.0.0 v7.7.0 v7.8.0 labels Mar 25, 2020
@javanna javanna requested a review from jimczi March 25, 2020 15:43
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (:Search/Search)

@bpintea
Copy link
Contributor

bpintea commented Mar 25, 2020

7.7 has been FFrozen, can the v7.7.0 label be removed?

@javanna
Copy link
Member Author

javanna commented Mar 25, 2020

@Mpdreamz can correct me if I am wrong, but I think that this a blocker for the language clients. As far as I understand wait_for_completion is expected to be a boolean like in other API and breaks some of the clients, which is why it was decided to rename it.

@bpintea
Copy link
Contributor

bpintea commented Mar 25, 2020

Thanks, @javanna!

@Mpdreamz can correct me if I am wrong, but I think that this a blocker for the language clients.

@jpountz May I update the list of blocking issues to include this one?

@Mpdreamz
Copy link
Member

It won't break the clients (unless it was an enum) but these type of changes are incredibly hard to patch after a release. Since this was discussed and agreed upon by a large group on the issueI would vote to raise it as blocker so that it can go in the next bc. Similar to #54196

@jpountz
Copy link
Contributor

jpountz commented Mar 25, 2020

+1 to merge to 7.7. I replaced the >enhancement label with >non-issue since async search has not been released yet.

Copy link
Contributor

@jimczi jimczi left a comment

Choose a reason for hiding this comment

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

I left two comments, LGTM otherwise

@javanna javanna merged commit 1c48214 into elastic:master Mar 26, 2020
javanna added a commit that referenced this pull request Mar 26, 2020
This commit renames wait_for_completion to wait_for_completion_timeout in submit async search and get async search.
Also it renames clean_on_completion to keep_on_completion and turns around its behaviour.

Closes #54069
@javanna
Copy link
Member Author

javanna commented Mar 26, 2020

heads up @lukasolson you will have to rename waitForCompletion to waitForCompletionTimeout in your code. I believe Kibana is not using cleanOnCompletion hence no change needed there.

javanna added a commit that referenced this pull request Mar 26, 2020
This commit renames wait_for_completion to wait_for_completion_timeout in submit async search and get async search.
Also it renames clean_on_completion to keep_on_completion and turns around its behaviour.

Closes #54069
@lizozom
Copy link

lizozom commented Apr 1, 2020

@lukasolson @javanna was this backported to 7.7?

@javanna
Copy link
Member Author

javanna commented Apr 1, 2020

@lizozom yes it was, see 2814226

@lizozom
Copy link

lizozom commented Apr 1, 2020

@javanna I'm asking, because backporting the kibana change to 7.7 fails in testing, while the 7.x and the 8.0 branches were working and merged

elastic/kibana#61917

@lizozom
Copy link

lizozom commented Apr 1, 2020

@javanna I think I might have found the issue. Lets see how the build goes!

@javanna
Copy link
Member Author

javanna commented Apr 1, 2020

@lizozom I see, let me know if I can do anything to help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>non-issue :Search/Search Search-related issues that do not fall into other categories v7.7.0 v7.8.0 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clarify async search REST parameters

8 participants