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

MultiGet: Fail when using no routing on an alias to an index that requires routing #7145

Closed

Conversation

Projects
None yet
4 participants
@javanna
Copy link
Member

javanna commented Aug 4, 2014

The multi_get api and multi_term_vector apis should always return an error when trying to get a document without routing if the routing is set to required. Yet, when using an alias a failure is not returned.

The problem is that theroutingRequired check needs to be done passing in the resolved concrete index.

@javanna javanna added bug labels Aug 4, 2014

@javanna javanna self-assigned this Aug 4, 2014

@kimchy

This comment has been minimized.

Copy link
Member

kimchy commented Aug 4, 2014

LGTM

MultiGet & MultiTermVector api: fail when using no routing and an ali…
…as to an index that has routing required (for that doc type)

Made sure that the routing required check is performed against the concrete index, added use of aliases to existing routing tests.

Taken the change to unify the failure message as well to this form: routing is required for [" + index + "]/[" + type + "]/[" + id + "]
@javanna

This comment has been minimized.

Copy link
Member Author

javanna commented Aug 4, 2014

Sorry @kimchy I updated the PR post LGTM :) found the same bug in multi term vector api and improved tests. Mind having another look?

@kimchy

This comment has been minimized.

Copy link
Member

kimchy commented Aug 4, 2014

I see, it was missing concrete index, LGTM

@javanna javanna closed this in 8989d06 Aug 4, 2014

javanna added a commit that referenced this pull request Aug 4, 2014

MultiGet & MultiTermVector api: fail when using no routing and an ali…
…as to an index that has routing required (for that doc type)

Made sure that the routing required check is performed against the concrete index, added use of aliases to existing routing tests.

Taken the change to unify the failure message as well to this form: routing is required for [" + index + "]/[" + type + "]/[" + id + "]

Closes #7145

javanna added a commit that referenced this pull request Aug 4, 2014

MultiGet & MultiTermVector api: fail when using no routing and an ali…
…as to an index that has routing required (for that doc type)

Made sure that the routing required check is performed against the concrete index, added use of aliases to existing routing tests.

Taken the change to unify the failure message as well to this form: routing is required for [" + index + "]/[" + type + "]/[" + id + "]

Closes #7145

@jpountz jpountz removed the review label Aug 11, 2014

javanna added a commit that referenced this pull request Sep 8, 2014

MultiGet & MultiTermVector api: fail when using no routing and an ali…
…as to an index that has routing required (for that doc type)

Made sure that the routing required check is performed against the concrete index, added use of aliases to existing routing tests.

Taken the change to unify the failure message as well to this form: routing is required for [" + index + "]/[" + type + "]/[" + id + "]

Closes #7145

@clintongormley clintongormley changed the title MultiGet Api: fail when using no routing and an alias to an index that h... MultiGet: Fail when using no routing on an alias to an index that requires routing Sep 8, 2014

mute pushed a commit to mute/elasticsearch that referenced this pull request Jul 29, 2015

MultiGet & MultiTermVector api: fail when using no routing and an ali…
…as to an index that has routing required (for that doc type)

Made sure that the routing required check is performed against the concrete index, added use of aliases to existing routing tests.

Taken the change to unify the failure message as well to this form: routing is required for [" + index + "]/[" + type + "]/[" + id + "]

Closes elastic#7145
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.