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

[ML] Remove all SearchResponse direct references from tests #102024

Merged

Conversation

benwtrent
Copy link
Member

Moving towards ref counted search responses, this removes all direct references to SearchResponse from ML tests.

The remaining references in ML tests are for mocked search responses.

Additionally, while making this change, I noticed there are multiple places in ML production code that have the SearchResponse searchResponse = pattern. Meaning, once we refCount SearchResponse, those places will have to be updated as well (in a future PR).

Related to: #100966
Related to: #99590

@benwtrent benwtrent added >test Issues or PRs that are addressing/adding tests :ml Machine learning v8.12.0 labels Nov 10, 2023
@elasticsearchmachine elasticsearchmachine added the Team:ML Meta label for the ML team label Nov 10, 2023
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/ml-core (Team:ML)

Copy link
Contributor

@droberts195 droberts195 left a comment

Choose a reason for hiding this comment

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

LGTM

@benwtrent benwtrent changed the title Remove all SearchResponse direct references from tests [ML] Remove all SearchResponse direct references from tests Nov 10, 2023
@benwtrent benwtrent added the auto-merge Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Nov 10, 2023
@elasticsearchmachine elasticsearchmachine merged commit 8964b87 into elastic:main Nov 10, 2023
13 checks passed
@benwtrent benwtrent deleted the tests/remove-search-response-ref branch November 10, 2023 16:38
davidkyle pushed a commit to davidkyle/elasticsearch that referenced this pull request Nov 13, 2023
Moving towards ref counted search responses, this removes all direct
references to SearchResponse from ML tests.

The remaining references in ML tests are for mocked search responses.

Additionally, while making this change, I noticed there are multiple
places in ML production code that have the `SearchResponse
searchResponse = ` pattern. Meaning, once we refCount `SearchResponse`,
those places will have to be updated as well (in a future PR).

Related to: elastic#100966 Related
to: elastic#99590
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :ml Machine learning Team:ML Meta label for the ML team >test Issues or PRs that are addressing/adding tests v8.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants