-
Notifications
You must be signed in to change notification settings - Fork 53
add reindex api example #13
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
Conversation
Thanks @Yogendra0Sharma ! |
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.
Hi! Thanks a lot for your PR, here is just some minor changes to make.
index.html
Outdated
<a name="reIndexes"></a> | ||
<h2 class="ui dividing header">Reindex API</h2> | ||
<h3 class="ui header">Simple Reindex Operation</h3> | ||
<pre class="es1 es2"><code>POST /_reindex |
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.
The ReIndex API was not available in Elasticsearch 1, so the es1 class should not be set here. In fact the whole "reindex" part should be enclosed in a div with "es2 es5" classes.
index.html
Outdated
"index": "test-index-new" | ||
} | ||
}</code></pre> | ||
<pre class="es5"><code>POST /_reindex |
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.
Same here, this example is compatible with es2 too.
index.html
Outdated
"type": "female" | ||
} | ||
} | ||
} |
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 like your JSON is not valid here?
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, it's a typo. I have resolved this issue and submitted the change. please review.
} | ||
}</code></pre> | ||
<h3 class="ui header">Selective Reindex Operation</h3> | ||
<p > |
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.
Can you remove the extra space?
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.
sure.
index.html
Outdated
|
||
|
||
<div class="es2 es5"> | ||
<a name="reIndexes"></a> |
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.
As we do not add an entry in the menu, this can be removed too.
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 we have to add a reIndex option to the menu ? or just remove this code?
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.
You should remove this option, we don't have enough space in the menu 😬
Thanks for the JSON typo, can you look at my other comments please? |
✅ Hey, thank you @Yogendra0Sharma! 🙌 |
No description provided.