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

Java issues batch 8 #2959

Merged
merged 5 commits into from
Oct 1, 2024
Merged

Java issues batch 8 #2959

merged 5 commits into from
Oct 1, 2024

Conversation

l-trotta
Copy link
Contributor

  • 877: some optional values in ngram and edge tokenizer (details and server code references in the issue).
  • 880: ForceMergeRequest accepts wait_for_competion, so the response should be optional to handle both wait_for_competion cases.
  • 881: added missing allow_custom_routing to PutIndexTemplate. server code - failure_store is internal.
  • 883: we use StepKey as both current_step and next_step since they have the same fields, but in next_step action and name are optional. not super clear from the server code (it's all String), but the documentation confirms it (and DevTools test as well).

Everything should be safe to backport to 8.x and 8.15

Copy link
Contributor

Following you can find the validation results for the APIs you have changed.

API Status Request Response
ilm.move_to_step Missing test Missing test
indices.add_block 🟢 2/2 2/2
indices.analyze 🟢 242/242 242/242
indices.clear_cache 🟢 4/4 4/4
indices.clone 🟢 5/5 5/5
indices.close 🟢 49/49 49/49
indices.create_data_stream 🟢 29/29 29/29
indices.create 🔴 1039/1127 1127/1127
indices.data_streams_stats 🟢 4/4 4/4
indices.delete_alias 🔴 15/15 5/15
indices.delete_data_lifecycle Missing test Missing test
indices.delete_data_stream 🟢 28/28 28/28
indices.delete_index_template 🟢 1/1 1/1
indices.delete_template 🟢 9/9 9/9
indices.delete 🟢 105/105 105/105
indices.disk_usage 🟢 5/5 5/5
indices.downsample 🟢 1/1 1/1
indices.exists_alias 🟢 38/38 38/38
indices.exists_index_template Missing test Missing test
indices.exists_template 🟢 15/15 15/15
indices.exists 🟢 39/39 39/39
indices.explain_data_lifecycle Missing test Missing test
indices.field_usage_stats 🟢 5/5 5/5
indices.flush 🟢 40/40 40/40
indices.forcemerge 🟢 12/12 12/12
indices.get_alias 🔴 86/86 74/86
indices.get_data_lifecycle Missing test Missing test
indices.get_data_stream 🟢 11/11 11/11
indices.get_field_mapping 🟢 15/15 15/15
indices.get_index_template 🟢 11/11 11/11
indices.get_mapping 🔴 135/135 133/135
indices.get_settings 🔴 62/62 60/62
indices.get_template 🟢 30/30 30/30
indices.get 🟢 53/53 53/53
indices.migrate_to_data_stream Missing test Missing test
indices.modify_data_stream Missing test Missing test
indices.open 🟢 20/20 20/20
indices.promote_data_stream Missing test Missing test
indices.put_alias 🔴 59/59 20/59
indices.put_data_lifecycle Missing test Missing test
indices.put_index_template 🔴 52/62 62/62
indices.put_mapping 🔴 111/115 115/115
indices.put_settings 🔴 54/57 57/57
indices.put_template 🔴 45/46 46/46
indices.recovery 🟢 9/9 9/9
indices.refresh 🟢 265/265 265/265
indices.reload_search_analyzers 🟢 2/2 2/2
indices.resolve_cluster 🔴 10/12 12/12
indices.resolve_index 🟢 6/6 6/6
indices.rollover 🔴 26/26 1/26
indices.segments 🔴 6/6 5/6
indices.shard_stores 🔴 5/5 4/5
indices.shrink 🟢 5/5 5/5
indices.simulate_index_template 🔴 6/7 5/7
indices.simulate_template 🔴 5/5 4/5
indices.split 🟢 11/11 11/11
indices.stats 🔴 112/112 33/111
indices.unfreeze 🟢 1/1 1/1
indices.update_aliases 🔴 26/26 7/26
indices.validate_query 🟢 12/12 12/12

You can validate these APIs yourself by using the make validate target.

Copy link
Member

@pquentin pquentin left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM. Sorry that I missed it earlier.

Comment on lines 20 to 25
export class StepKey {
action: string
name: string
// action and name are optional in case they are used in next_step
action?: string
name?: string
phase: string
}
Copy link
Member

Choose a reason for hiding this comment

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

Should we make a NextStepKey class? Just wondering, really.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

was wondering the same, that would be a breaking change though and it would have to wait

Copy link
Member

Choose a reason for hiding this comment

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

Agreed! Can you please add it to #2973?

And feel free to merge this pull request

Copy link
Member

Choose a reason for hiding this comment

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

Not 100% related, but juding from the documentation, it seems that both, current_step and next_step should be required.

We currently model them as optional:

/**
 * @rest_spec_name ilm.move_to_step
 * @availability stack since=6.6.0 stability=stable
 */
export interface Request extends RequestBase {
  path_parts: {
    index: IndexName
  }
  body: {
    current_step?: StepKey
    next_step?: StepKey
  }
}

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 catch! I'll remove the optional

Copy link
Contributor

github-actions bot commented Oct 1, 2024

Following you can find the validation results for the APIs you have changed.

API Status Request Response
ilm.move_to_step Missing test Missing test
indices.add_block 🟢 2/2 2/2
indices.analyze 🟢 242/242 242/242
indices.clear_cache 🟢 4/4 4/4
indices.clone 🟢 5/5 5/5
indices.close 🟢 49/49 49/49
indices.create_data_stream 🟢 29/29 29/29
indices.create 🔴 1040/1126 1126/1126
indices.data_streams_stats 🟢 4/4 4/4
indices.delete_alias 🔴 15/15 5/15
indices.delete_data_lifecycle Missing test Missing test
indices.delete_data_stream 🟢 28/28 28/28
indices.delete_index_template 🟢 1/1 1/1
indices.delete_template 🟢 9/9 9/9
indices.delete 🟢 105/105 105/105
indices.disk_usage 🟢 5/5 5/5
indices.downsample 🟢 1/1 1/1
indices.exists_alias 🟢 38/38 38/38
indices.exists_index_template Missing test Missing test
indices.exists_template 🟢 15/15 15/15
indices.exists 🟢 39/39 39/39
indices.explain_data_lifecycle Missing test Missing test
indices.field_usage_stats 🟢 5/5 5/5
indices.flush 🟢 40/40 40/40
indices.forcemerge 🟢 12/12 12/12
indices.get_alias 🔴 86/86 74/86
indices.get_data_lifecycle Missing test Missing test
indices.get_data_stream 🟢 11/11 11/11
indices.get_field_mapping 🟢 15/15 15/15
indices.get_index_template 🟢 11/11 11/11
indices.get_mapping 🔴 135/135 133/135
indices.get_settings 🔴 62/62 60/62
indices.get_template 🟢 30/30 30/30
indices.get 🟢 53/53 53/53
indices.migrate_to_data_stream Missing test Missing test
indices.modify_data_stream Missing test Missing test
indices.open 🟢 20/20 20/20
indices.promote_data_stream Missing test Missing test
indices.put_alias 🔴 59/59 20/59
indices.put_data_lifecycle Missing test Missing test
indices.put_index_template 🔴 52/62 62/62
indices.put_mapping 🔴 111/115 115/115
indices.put_settings 🔴 54/57 57/57
indices.put_template 🔴 45/46 46/46
indices.recovery 🟢 9/9 9/9
indices.refresh 🟢 265/265 265/265
indices.reload_search_analyzers 🟢 2/2 2/2
indices.resolve_cluster 🔴 10/12 12/12
indices.resolve_index 🟢 6/6 6/6
indices.rollover 🔴 26/26 1/26
indices.segments 🔴 6/6 5/6
indices.shard_stores 🔴 5/5 4/5
indices.shrink 🟢 5/5 5/5
indices.simulate_index_template 🔴 6/7 5/7
indices.simulate_template 🔴 5/5 4/5
indices.split 🟢 11/11 11/11
indices.stats 🔴 112/112 33/111
indices.unfreeze 🟢 1/1 1/1
indices.update_aliases 🔴 26/26 7/26
indices.validate_query 🟢 12/12 12/12

You can validate these APIs yourself by using the make validate target.

@l-trotta l-trotta merged commit 3b1143a into main Oct 1, 2024
8 checks passed
@l-trotta l-trotta deleted the java-issues-batch-8 branch October 1, 2024 16:16
Copy link
Contributor

github-actions bot commented Oct 1, 2024

The backport to 8.15 failed:

The process '/usr/bin/git' failed with exit code 1

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-8.15 8.15
# Navigate to the new working tree
cd .worktrees/backport-8.15
# Create a new branch
git switch --create backport-2959-to-8.15
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 3b1143a5a96fc7596af96485bccca492798d6f0e
# Push it to GitHub
git push --set-upstream origin backport-2959-to-8.15
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-8.15

Then, create a pull request where the base branch is 8.15 and the compare/head branch is backport-2959-to-8.15.

github-actions bot pushed a commit that referenced this pull request Oct 1, 2024
* [877] ngram and edge tokenizer optional values

* [880] _shards optional because of wait_for_completion

* [881] added missing allow_custom_routing

* [883] action and name optional next_step

* both steps mandatory in move to step request

(cherry picked from commit 3b1143a)
l-trotta added a commit that referenced this pull request Oct 1, 2024
* [877] ngram and edge tokenizer optional values

* [880] _shards optional because of wait_for_completion

* [881] added missing allow_custom_routing

* [883] action and name optional next_step

* both steps mandatory in move to step request

(cherry picked from commit 3b1143a)

Co-authored-by: Laura Trotta <[email protected]>
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.

3 participants