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

[DOCS] Correct source.index parm def in reindex API #52004

Closed
wants to merge 1 commit into from
Closed

[DOCS] Correct source.index parm def in reindex API #52004

wants to merge 1 commit into from

Conversation

jrodewig
Copy link
Contributor

@jrodewig jrodewig commented Feb 6, 2020

Corrects the source.index request body parameter definition to
state that arrays, not comma-separated values, are accepted.

Relates to #51949

@jrodewig jrodewig added >docs General docs changes :Distributed/Reindex Issues relating to reindex that are not caused by issues further down labels Feb 6, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-docs (>docs)

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (:Distributed/Reindex)

@jrodewig jrodewig changed the title [DOCS] Correct _source.index parm def in reindex API [DOCS] Correct source.index parm def in reindex API Feb 6, 2020
@henningandersen
Copy link
Contributor

I think we should just support the "," notation instead to try to align with the multiple indices documentation. This is also how snapshot/restore works.

@henningandersen
Copy link
Contributor

Opened #52044 to fix this.

@jrodewig jrodewig changed the base branch from master to 7.6 February 7, 2020 13:47
Corrects the `source.index` request body parameter definition to
state that arrays, not comma-separated values, are accepted.
@jrodewig
Copy link
Contributor Author

jrodewig commented Feb 7, 2020

Thanks @henningandersen. However, I think this would still be a relevant docs update for 7.6 7.5 and 6.8.

I've changed the target branch and updated the labels appropriately.

Copy link
Contributor

@henningandersen henningandersen left a comment

Choose a reason for hiding this comment

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

LGTM.

Ideally, we should backport the fix from #52044 to 6.8 too, but given that there is such an easy workaround and the code changed a lot between 6.8 and 7.x, I think just updating the docs instead in 6.8 is fine.

I am not sure about 7.5 though, given that 7.6 is now released, I assume we should only do this for 6.8? The next 7.6 release will contain the fix.

@jrodewig jrodewig removed the v7.5.2 label Feb 19, 2020
@jrodewig jrodewig changed the base branch from 7.6 to 6.8 February 19, 2020 12:28
@jrodewig jrodewig changed the base branch from 6.8 to 7.6 February 19, 2020 12:32
@jrodewig
Copy link
Contributor Author

Thanks @henningandersen. After taking a closer look at the 6.8 docs, I believe this is already documented pretty well:

Screen Shot 2020-02-19 at 7 34 05 AM

I'll close this PR. Thanks for fixing this!

@jrodewig jrodewig closed this Feb 19, 2020
@jrodewig jrodewig deleted the patch__reindex-source-array branch February 19, 2020 12:34
@jrodewig jrodewig removed the v6.8.6 label Apr 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/Reindex Issues relating to reindex that are not caused by issues further down >docs General docs changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

reindex doesn't accept comma separated values as inputs
3 participants