-
Notifications
You must be signed in to change notification settings - Fork 84
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
Fix spec issues reported in the Java client #1723
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good to me, had one question about the Slices
type:
specification/_types/common.ts
Outdated
* Uses sliced scroll to parallelize process. Using `auto` chooses a reasonable number for most data streams | ||
* and indices. | ||
* | ||
* @codegen_names value, auto |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the name auto
here still make sense if another value is added to the enum?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does, auto
is a special value for this field which is translated to 0
on ES side. Providing 0
in the payload doesn't apply the same behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After a discussion with @technige we ended up considering that all
is the name of a function used to compute the number of slices. There's only one such function as of now, but there could be others. So this becomes:
/**
* Slices configuration used to parallelize a process.
*
* @codegen_names value, computed
*/
export type Slices = integer | SlicesCalculation
/**
* Named computations to calculate the number of slices
*/
export enum SlicesCalculation {
/**
* Let Elasticsearch choose a reasonable number for most data streams and indices.
*/
auto
}
Following you can find the validation results for the APIs you have changed.
You can validate these APIs yourself by using the |
The backport to
To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-7.17 7.17
# Navigate to the new working tree
cd .worktrees/backport-7.17
# Create a new branch
git switch --create backport-1723-to-7.17
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick --mainline 1 da53b6e6dc15c9a721c96aee24c4d25c0747fd6a
# Push it to GitHub
git push --set-upstream origin backport-1723-to-7.17
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-7.17 Then, create a pull request where the |
This PR fixes a number of API spec issues that were reported in the Java client. We're experimenting a batch processing of these issue, as they are most of the time very tiny problems such as missing fields or required fields that should be optional.
This PR has 2 larger changes: the new
dynamic_property
property type, and a refactoring ofHiglight
andHighlightField
with a common base class, as they have most attribute in common (same in the ES code base).ILM Policy has an invalid "name" field.
Found in Put Lifecycle method in ILM Client causes 400 Bad Request elasticsearch-java#160
CreateApiKeyRequest.RoleDescriptor is missing some fields. Fixes CreateApiKeyRequest.RoleDescriptor is missing some fields #1555.
Found in New Java client does not have all fields for CreateApiKeyRequest.RoleDescriptor elasticsearch-java#106
Missing support for {dynamic_type} property mapping. Fixes Missing support for {dynamic_type} property mapping #1558.
Found in Missing support for {dynamic_type} elasticsearch-java#142
Missing support for auto slicing.
Found in Missing support for auto slicing elasticsearch-java#147
RemoteSource username/password should be optional.
Found in
RemoteSource
requires optional parameters to be set. elasticsearch-java#152SuggestFuzziness requires optional attributes to be set.
Found in
SuggestFuzziness
requires optional attributes to be set. elasticsearch-java#162Missing "attributes" in explain token with custom analyzer.
Found in Analysis API with explain fails to parse "attributes" from plugin-installed analyzer elasticsearch-java#154
Missing fields in MultisearchBody
Found in msearch missing version and timeout elasticsearch-java#161, Multisearch requests in new Java API do not support “sort” or “searchAfter” properties elasticsearch-java#170 and MultisearchBody Should Support Highlight and Source elasticsearch-java#288
TermVectors.terms.tokens should be optional
Found in Missing required property 'Term.tokens' in Termvectors response. elasticsearch-java#184
Missing fields in Highlight (also refactored Highlight & HighlightField with a common base class)
Found in
force_source
andphrase_limit
parameters missing to build highlight queries elasticsearch-java#185