Deprecate types in termvector and mtermvector requests.#36182
Deprecate types in termvector and mtermvector requests.#36182jtibshirani merged 9 commits intoelastic:masterfrom
Conversation
|
Pinging @elastic/es-search |
a6f9c4b to
9516344
Compare
| "ids" : ["testing_document"] | ||
|
|
||
| - match: {docs.0.term_vectors.text.terms.brown.term_freq: 2} | ||
| - match: {docs.0.term_vectors.text.terms.brown.ttf: 2} |
There was a problem hiding this comment.
What is the expected behaviour when docs are stored with a custom type but queried via a typeless api?
Not sure there's a test for that here? (Ignore me - I've just seen that overlaps with #35790 )
There was a problem hiding this comment.
👍 the tests in that PR should cover this case.
| public class TermVectorsRequest extends SingleShardRequest<TermVectorsRequest> implements RealtimeRequest { | ||
| private static final DeprecationLogger deprecationLogger = new DeprecationLogger( | ||
| LogManager.getLogger(MultiTermVectorsRequest.class)); | ||
|
|
There was a problem hiding this comment.
Cut-and-paste code has wrong class?
There was a problem hiding this comment.
Oops, thanks for catching this!
| } else if (TYPE.match(currentFieldName, parser.getDeprecationHandler())) { | ||
| termVectorsRequest.type = parser.text(); | ||
| deprecationLogger.deprecated(RestMultiTermVectorsAction.TYPES_DEPRECATION_MESSAGE); | ||
| } else if (ID.match(currentFieldName, parser.getDeprecationHandler())) { |
There was a problem hiding this comment.
cut-and-paste code with wrong message?
There was a problem hiding this comment.
I'll update this too, as I think it's clearer to use RestTermVectorsAction here. I initially used the multi termvectors message because this method is also used in parsing multi termvector requests.
jpountz
left a comment
There was a problem hiding this comment.
LGTM minus the comment from Mark about the class used to instantiate a logger.
|
LGTM aside from logging comment and I see tests fail due to TermVectorsUnitTests missing a couple of |
|
@elasticmachine run default distro tests |
|
@elasticmachine run gradle build tests 2 |
The following updates were made:
Rest*TermVectorsAction, plus tests inRest*TermVectorsActionTests.I assumed here that we will be going with the endpoint format
{index}/_termvectors/{id}, but we should hold off on merging this until we've reached a final consensus.