-
Notifications
You must be signed in to change notification settings - Fork 186
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
[CCI] Create index lifecycle guide #362
[CCI] Create index lifecycle guide #362
Conversation
Signed-off-by: Alexei Karikov <karikov.alist.ru@gmail.com>
Thank you for contributing @Nicksqain! Can you add a Changelog entry for this? Also, I see you have 3 PRs for adding guides, you can also combine them into one with a single Changelog entry if that works. |
@VachaShah Hello! thanks for the feedback! I will definitely add an entry. If I add to one, will the others have a "skip changelog"? |
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## main #362 +/- ##
=======================================
Coverage 72.01% 72.01%
=======================================
Files 77 77
Lines 7190 7190
=======================================
Hits 5178 5178
Misses 2012 2012 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
If you keep the 3 separate PRs, then you can add Changelog entry in each PR. Or you can combine all 3 PRs into 1 and add a Changelog entry. For Changelog, you can add an entry in the Added section for Unreleased. |
Signed-off-by: Alexei Karikov <karikov.alist.ru@gmail.com>
Can we wait for the index lifecycle to merge first and then re-run the link checker? |
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.
Thank you @Nicksqain! I tried the code in the guide and had a few comments.
guides/index_lifecycle.md
Outdated
|
||
```python | ||
print(client.indices.exists(index='burner')) # => False | ||
client.create(index='burner', body={'lorem': 'ipsum'}) |
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.
This would be client.index
with id
parameter:
client.index(id='1', index='burner', body={'lorem': 'ipsum'})
guides/index_lifecycle.md
Outdated
We can also delete multiple indices at once: | ||
|
||
```python | ||
client.indices.delete(index=['movies', 'paintings', 'burner'], ignore=[404]) |
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.
ignore
is not an option for python. So this would be:
print(client.indices.delete(index=['paintings', 'burner']))
since movies
is deleted in the previous line.
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.
But other guides for clients have the presence of ignoring. This is part of the guide.
Did I make a mistake or is it just that there is no option to ignore the 404 error?
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.
Looking at the code, the parameter available for ignoring non-existing indices is ignore_unavailable
. I tried it and the following works:
print(client.indices.delete(index=['art', 'paintings', 'burner'], ignore_unavailable=True))
where art
index does not exist. The response is:
{'acknowledged': True}
Lets change it to ignore_unavailable
.
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.
Yes, thanks for the correction! I had my doubts, as the rust client also had this option.
guides/index_lifecycle.md
Outdated
client.indices.delete(index=['movies', 'paintings', 'burner'], ignore=[404]) | ||
``` | ||
|
||
Notice that we are passing `ignore: 404` to the request. This tells the client to ignore the `404` error if the index doesn't exist for deletion. Without it, the above `delete` request will throw an error because the `movies` index has already been deleted in the previous example. |
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.
This section is not applicable since ignore
is not an option for this client.
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.
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.
ignore_unavailable
seems to be the correct option. See
opensearch-py/opensearchpy/client/indices.py
Line 295 in 03b72c3
:arg ignore_unavailable: Ignore unavailable indexes (default: |
Sure, we can merge this PR first and you can fix the changelogs one by one for your PRs after that. I just added a few comments for this PR. |
Signed-off-by: Alexei Karikov <karikov.alist.ru@gmail.com>
@VachaShah I've added changes! Happy to help! |
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.
This looks good! Can you pull the latest changes from main and resolve the conflicts in the Changelog? We should be good to merge then!
Signed-off-by: Alexei Karikov <karikov.alist.ru@gmail.com>
Signed-off-by: Alexei Karikov <karikov.alist.ru@gmail.com>
@VachaShah |
Signed-off-by: Alexei Karikov <karikov.alist.ru@gmail.com>
* Create index lifecycle guide Signed-off-by: Alexei Karikov <karikov.alist.ru@gmail.com> * Add changelog entry Signed-off-by: Alexei Karikov <karikov.alist.ru@gmail.com> * Replace ignore param and add short corrections Signed-off-by: Alexei Karikov <karikov.alist.ru@gmail.com> * reset spaces Signed-off-by: Alexei Karikov <karikov.alist.ru@gmail.com> * Correct indents Signed-off-by: Alexei Karikov <karikov.alist.ru@gmail.com> --------- Signed-off-by: Alexei Karikov <karikov.alist.ru@gmail.com> Signed-off-by: Raman Saparkhan <romasaparhan19@gmail.com>
* Create index lifecycle guide Signed-off-by: Alexei Karikov <karikov.alist.ru@gmail.com> * Add changelog entry Signed-off-by: Alexei Karikov <karikov.alist.ru@gmail.com> * Replace ignore param and add short corrections Signed-off-by: Alexei Karikov <karikov.alist.ru@gmail.com> * reset spaces Signed-off-by: Alexei Karikov <karikov.alist.ru@gmail.com> * Correct indents Signed-off-by: Alexei Karikov <karikov.alist.ru@gmail.com> --------- Signed-off-by: Alexei Karikov <karikov.alist.ru@gmail.com> Signed-off-by: Raman Saparkhan <romasaparhan19@gmail.com>
Description
Issues Resolved
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.