-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
Use tsdb's id in Engine tests #85055
Conversation
Pinging @elastic/es-analytics-geo (Team:Analytics) |
This should be enough to detect if tsdb's `_id` field mapper changes enough to cause trouble for `Engine`. I suspect that in the end we'll need something more like the changes that elastic#84996 did for RecoverySourceHandlerTests but that's a much bigger change that I'd prefer to hold back until we need it. If tsdb's `_id` field mapper changes enough to cause trouble for `Engine`.
final Field uidField = new Field( | ||
"_id", | ||
id, | ||
randomBoolean() ? ProvidedIdFieldMapper.Defaults.FIELD_TYPE : TsidExtractingIdFieldMapper.FIELD_TYPE |
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.
I'm thinking I should pick this up front rather than per document....
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.
I'll do that now.
run |
run elasticsearch-ci/part-1 |
Well, this grew larger than I'd hoped. But it's still smaller than it'd be if we needed to use the whole parsing infrastructure. I'd like to try this for now and bite the bullet on the parsing infrastructure only if we need it. This should be enough to detect if the field changes in an incompatible way. |
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.
LGTM.
I wonder if it will eventually bite us that the _id
s do not conform to the field mapper. But we can tackle that when we have to.
I'm reasonably sure we'll have to, yeah. But this'll catch those cases at least. Which'll be better than flying blind until then. |
This should be enough to detect if tsdb's
_id
field mapper changesenough to cause trouble for
Engine
. I suspect that in the end we'llneed something more like the changes that #84996 did for
RecoverySourceHandlerTests but that's a much bigger change that I'd
prefer to hold back until we need it. If tsdb's
_id
field mapperchanges enough to cause trouble for
Engine
.