Move required routing validation to IndexRouting#79472
Move required routing validation to IndexRouting#79472elasticsearchmachine merged 13 commits intoelastic:masterfrom
Conversation
|
Pinging @elastic/es-distributed (Team:Distributed) |
Now that we have a place responsible for picking shards we can move the "routing is required" validation there, right next to the validation for the `routing_path` stuff. This feels like a more convenient place to have it. Certainly a nice place to unit test it.
|
run elasticsearch-ci/part-1 |
henningandersen
left a comment
There was a problem hiding this comment.
LGTM with one change. Thanks for improving this.
| termVectorsRequest.id(), termVectorsRequest.routing()); | ||
| } catch (Exception e) { | ||
| responses.set(i, new MultiTermVectorsItemResponse(null, | ||
| new MultiTermVectorsResponse.Failure(termVectorsRequest.index(), termVectorsRequest.id(), e))); |
There was a problem hiding this comment.
I think this changes the way we report a missing routing to list the original request index (which could be an alias) rather than the concrete index. I think we want to report the concrete index here?
Same in TransportMultiGetAction.
I suppose one could argue that reporting the original index-name is just as good since the info is baked into the exception message. But it does open a discussion on this being a breaking change (though a very small one)?
There was a problem hiding this comment.
But it does open a discussion on this being a breaking change (though a very small one)?
I don't think we consider error messages to be part of the semver contract. But changing it without thinking about it is not good. I'll have a closer look.
We fail to build the exception in 7.x. Big yikes.
…uting_from_source_clean_2
…uting_from_source_clean_2
|
@elasticmachine update branch |
0d1363d to
673d8bf
Compare
Now that we have elastic#79394 and elastic#79472 the switch statement in `TransportBulkAction` is mostly just assigning routing and doing some stuff with INDEX and UPDATE requests. This pulls the routing stuff into a single call and calls out the index-only stuff. Now, at least, it's clearer that only INDEX and UPDATE are special.
Now that we have #79394 and #79472 the switch statement in `TransportBulkAction` is mostly just assigning routing and doing some stuff with INDEX and UPDATE requests. This pulls the routing stuff into a single call and calls out the index-only stuff. Now, at least, it's clearer that only INDEX and UPDATE are special.
Now that we have a place responsible for picking shards we can move the
"routing is required" validation there, right next to the validation for
the
routing_pathstuff. This feels like a more convenient place tohave it. Certainly a nice place to unit test it.