-
Notifications
You must be signed in to change notification settings - Fork 185
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 advanced index actions guide #363
[CCI] Create advanced index actions guide #363
Conversation
Signed-off-by: Alexei Karikov <[email protected]>
@Nicksqain can you add a change in the CHANGELOG file? |
Codecov Report
@@ Coverage Diff @@
## main #363 +/- ##
=======================================
Coverage 70.92% 70.92%
=======================================
Files 81 81
Lines 7732 7732
=======================================
Hits 5484 5484
Misses 2248 2248 |
@harshavamsi Could you suggest which section and version?
|
Signed-off-by: Alexei Karikov <[email protected]>
@Nicksqain, please add changelog to unreleased - added section. And also merge all the commits into single commit. |
Code tested and Its working. |
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.
We've made some improvements to the users guide since this.
- Resolve conflicts.
- Link from users guide.
- Add a working sample.
Let's finish this!
@dblock Yes, I'll be sure to add the necessary changes on a free day! |
@Nicksqain, Could you please make the requested changes when you have a moment? Thank you! |
Signed-off-by: Alexei Karikov <[email protected]>
Signed-off-by: Alexei Karikov <[email protected]>
Signed-off-by: Alexei Karikov <[email protected]>
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.
Looks good, let's fix capitalization?
We've also started adding working samples that are mentioned in guides to avoid copy-paste errors. Consider writing the code that's described in this guide in https://github.com/opensearch-project/opensearch-py/tree/main/samples?
USER_GUIDE.md
Outdated
@@ -153,6 +153,7 @@ print(response) | |||
- [Search](guides/search.md) | |||
- [Point in Time](guides/point_in_time.md) | |||
- [Using a Proxy](guides/proxy.md) | |||
- [Advanced index actions](guides/advanced_index_actions.md) |
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.
Capitalize Index Actions
guides/advanced_index_actions.md
Outdated
|
||
## API Actions | ||
|
||
### Clear index cache |
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.
Do other guides capitalize Clear Index Cache?
Signed-off-by: Alexei Karikov <[email protected]>
Signed-off-by: Alexei Karikov <[email protected]>
guides/advanced_index_actions.md
Outdated
- [Open/Close Index](#open-close-index) | ||
- [Force merge index](#force-merge-index) | ||
- [Clone index](#clone-index) | ||
- [Split index](#split-index) |
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.
Capitalize Split index, Clone index, Force merge index
Signed-off-by: Alexei Karikov <[email protected]>
guides/advanced_index_actions.md
Outdated
client.indices.open(index='movies') | ||
``` | ||
|
||
### Force merge index |
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.
One more.
@Nicksqain, Thanks for your contribution. Please add a working sample if possible. |
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.
Will you write a sample as part of this PR?
Signed-off-by: Alexei Karikov <[email protected]>
@dblock |
We can merge the guide, thanks for your help. |
@saimedhi you're good with this? |
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.
@Nicksqain, Thank you for contributing :) Could you kindly make those two small changes mentioned below?
I tested the code its working fine. And we can merge this PR after those 2 small changes. Thanks!
- [Clear Index Cache](#clear-index-cache) | ||
- [Flush Index](#flush-index) | ||
- [Refresh Index](#refresh-index) | ||
- [Open/Close Index](#open-close-index) |
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 link is not working.
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.
@dblock This guide has broken linking because of the use of [Open/Close Index](#open-close-index)
syntax. I think the /
character is breaking the #open-close-index
link, but not sure. Regardless, I need to change the section title to use a Open or close index
& make the hash link #open-or-close-index
.(or something similar)
- This isn't an issue in the Ruby guide version because those guides don't include the MD hash linking at the top of the guide
Is the section title I suggested okay to you? Or would you prefer some other title/hash-link?
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.
Perfectly fine!
client.indices.clear_cache(index='movies') | ||
``` | ||
|
||
By default, the `indices.clear_cache` API action clears all types of cache. To clear specific types of cache pass the the `query`, `fielddata`, or `request` parameter to the API action: |
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.
remove one "the" after cache pass.
@Nicksqain, kindly make the above changes and lets merge this :) |
@Nicksqain, kindly make the above changes and lets merge this :) |
@Nicksqain, kindly make the above changes and resolve merge conflicts. Thanks :) |
@Djcarrillo6 please do it! |
@dblock ok great I'm on it! |
@Djcarrillo6, please consider committing small fixes to the Nicksqain:advanced-index-actions-py-guide branch instead of creating new PR. If needed, create a separate PR for sample. Thank you. |
@saimedhi I opened a PR with my fixes/additions for this issue into @Nicksqain's open PR(this one) however I'm realizing now that he would have to accept it for those changes to be merged into this PR. Unless I'm mistaken, I don't think I can get my commit directly to his Nicksqain:advanced-index-actions-py-guide branch. Is there another way to get my branch into "Nicksqain:advanced-index-actions-py-guide"? |
I am sorry, you are right. please reopen your PR with all the changes. |
No worries! It will be opened shortly! |
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.