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] Get ML filters size should default to 100 #54207

Conversation

dimitris-athanasiou
Copy link
Contributor

@dimitris-athanasiou dimitris-athanasiou commented Mar 25, 2020

When get filters is called without setting the size
parameter only up to 10 filters are returned. However,
100 filters should be returned. This commit fixes this
and adds an integ test to guard it.

It seems this was accidentally broken in #39976.

Closes #54206

When get filters is called without setting the `size`
paramter only up to 10 filters are returned. However,
100 filters should be returned. This commit fixes this
and adds an integ test to guard it.

It seems this was accidentally broken in elastic#39976.

Closes elastic#54206
@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core (:ml)

// nothing to clean
}

public void testGetFilters_ShouldReturnUpTo100ByDefault() {
Copy link
Member

Choose a reason for hiding this comment

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

#nit
This seems like a very expensive test. Spinning up 3 nodes and native processes to make filter API calls.

I suppose it is thorough. Might could move to our single node internal cluster tests at some point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I fixed it.

@dimitris-athanasiou dimitris-athanasiou merged commit 98798a2 into elastic:master Mar 26, 2020
@dimitris-athanasiou dimitris-athanasiou deleted the get-ml-filters-size-should-default-to-100 branch March 26, 2020 14:51
dimitris-athanasiou added a commit to dimitris-athanasiou/elasticsearch that referenced this pull request Mar 26, 2020
When get filters is called without setting the `size`
paramter only up to 10 filters are returned. However,
100 filters should be returned. This commit fixes this
and adds an integ test to guard it.

It seems this was accidentally broken in elastic#39976.

Closes elastic#54206

Backport of elastic#54207
dimitris-athanasiou added a commit that referenced this pull request Mar 26, 2020
When get filters is called without setting the `size`
paramter only up to 10 filters are returned. However,
100 filters should be returned. This commit fixes this
and adds an integ test to guard it.

It seems this was accidentally broken in #39976.

Closes #54206

Backport of #54207
dimitris-athanasiou added a commit that referenced this pull request Mar 26, 2020
When get filters is called without setting the `size`
paramter only up to 10 filters are returned. However,
100 filters should be returned. This commit fixes this
and adds an integ test to guard it.

It seems this was accidentally broken in #39976.

Closes #54206

Backport of #54207
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.

[ML] Get filters size defaults to 10 instead of 100
4 participants