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

[HealthAPI] Add size parameter that controls the number of affected resources returned #92399

Merged
merged 5 commits into from
Dec 16, 2022

Conversation

andreidan
Copy link
Contributor

@andreidan andreidan commented Dec 15, 2022

This adds a size parameter that controls the maximum number of returned affected resources. The parameter defaults to 1000and must be positive

Closes #91930

…esources returned

This adds a `size` parameter that controls the maximum number of
returned affected resources. The parameter defaults to `1000`, must be
positive, and less than `10_000`
@github-actions
Copy link

Documentation preview:

@elasticsearchmachine elasticsearchmachine added the Team:Data Management Meta label for data/management team label Dec 15, 2022
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

@elasticsearchmachine
Copy link
Collaborator

Hi @andreidan, I've created a changelog YAML for you.

Copy link
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

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

LGTM, I left one question about limiting the maximum size, what do you think?

Comment on lines 168 to 170
} else if (size > 10_000) {
validationException = addValidationError("Cannot request more than 10_000 affected resources", validationException);
}
Copy link
Member

Choose a reason for hiding this comment

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

It seems a little strange to constrain this artificially, I understand having a reasonable default, but we don't limit the max number of search results or other places.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, that's a good point. Especially now that we chunk the response the clients will have a better experience with larger responses anyway.

but we don't limit the max number of search results or other places.

I guess we sort of do in the context of pagination https://www.elastic.co/guide/en/elasticsearch/reference/current/index-modules.html#index-max-result-window, but maybe we should cross that bridge when/if we get there.

I'll drop the upper bound

@andreidan
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/packaging-tests-unix-sample

@andreidan andreidan merged commit 3723af3 into elastic:main Dec 16, 2022
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.

[HealthAPI] Add size parameter that controls the number of affected resources returned
3 participants