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

Fix wait_for_completion option description and default setting docs #8136

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

epugh
Copy link
Collaborator

@epugh epugh commented Aug 30, 2024

Description

The documentation for the wait_for_completion turns out to be wrong, the default value is true. However, more importantly, it's not a great doc... I didn't get what it did at first till a colleague pointed it out and then someone on #general clarified it for me!

I did a quick search for wait_for_completion across docs, and stylisticially it's documented in a few ways. For my fix, I took the descirption used in delete-by-query.md.

I did NOT try to fix the other places where maybe the option description could be improved or standardized....

Issues Resolved

List any issues this PR will resolve, e.g. Closes [...].

Version

2.16

Checklist

  • [x ] By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and subject to the Developers Certificate of Origin.
    For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Copy link

Thank you for submitting your PR. The PR states are In progress (or Draft) -> Tech review -> Doc review -> Editorial review -> Merged.

Before you submit your PR for doc review, make sure the content is technically accurate. If you need help finding a tech reviewer, tag a maintainer.

When you're ready for doc review, tag the assignee of this PR. The doc reviewer may push edits to the PR directly or leave comments and editorial suggestions for you to address (let us know in a comment if you have a preference). The doc reviewer will arrange for an editorial review.

@epugh epugh force-pushed the fix_wait_for_completion_doc branch from 1061550 to 6219d97 Compare August 30, 2024 17:01
@epugh epugh changed the title reuse description from delete-by-query.md, to be clearer. fixes error Fix wait_for_completion option description and default setting docs Aug 30, 2024
Copy link
Collaborator

@Naarcha-AWS Naarcha-AWS left a comment

Choose a reason for hiding this comment

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

@epugh: Is the default setting false? Also, can you make sure that my suggestion doesn't change any technical meaning?

@@ -44,7 +44,7 @@ Parameter | Type | Description
refresh | Boolean | If true, OpenSearch refreshes shards to make the reindex operation available to search results. Valid options are `true`, `false`, and `wait_for`, which tells OpenSearch to wait for a refresh before executing the operation. Default is `false`.
timeout | Time | How long to wait for a response from the cluster. Default is `30s`.
wait_for_active_shards | String | The number of active shards that must be available before OpenSearch processes the reindex request. Default is 1 (only the primary shard). Set to `all` or a positive integer. Values greater than 1 require replicas. For example, if you specify a value of 3, the index must have two replicas distributed across two additional nodes for the operation to succeed.
wait_for_completion | Boolean | Waits for the matching tasks to complete. Default is `false`.
wait_for_completion | Boolean | Setting this parameter to false indicates to OpenSearch it should not wait for completion and perform this request asynchronously. Asynchronous requests run in the background, and you can use the [Tasks]({{site.url}}{{site.baseurl}}/api-reference/tasks) API to monitor progress.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
wait_for_completion | Boolean | Setting this parameter to false indicates to OpenSearch it should not wait for completion and perform this request asynchronously. Asynchronous requests run in the background, and you can use the [Tasks]({{site.url}}{{site.baseurl}}/api-reference/tasks) API to monitor progress.
wait_for_completion | Boolean | When `false`, OpenSearch does not wait for the reindex to complete and instead performs this request asynchronously. Asynchronous requests run in the background. You can use the [Tasks]({{site.url}}{{site.baseurl}}/api-reference/tasks/) API to monitor progress.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

so your change is good. However, it isn't clear that the default is "true", I thought the other wording maybe did that better. Thoughts on how to make it clear the default for this parameter is true?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To be clear, the default is "true"!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
wait_for_completion | Boolean | Setting this parameter to false indicates to OpenSearch it should not wait for completion and perform this request asynchronously. Asynchronous requests run in the background, and you can use the [Tasks]({{site.url}}{{site.baseurl}}/api-reference/tasks) API to monitor progress.
wait_for_completion | Boolean | When `false`, OpenSearch does not wait for the reindex to complete and instead performs this request asynchronously. Asynchronous requests run in the background. You can use the [Tasks]({{site.url}}{{site.baseurl}}/api-reference/tasks/) API to monitor progress. Default is `true`.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you can tag the default to the end unless you want to add additional context for the default behavior.

@Naarcha-AWS Naarcha-AWS added 4 - Doc review PR: Doc review in progress backport 2.17 Backport for version 2.17 and removed backport 2.16 labels Sep 11, 2024
@Naarcha-AWS
Copy link
Collaborator

@epugh: Let me know once you've implemented the changes and we'll get this merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - Doc review PR: Doc review in progress backport 2.17 Backport for version 2.17
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants