Skip to content

Conversation

@benwtrent
Copy link
Member

Since request_id is effectively required for all messages sent to the native process, this bubbles up the request_id to the outer object.

It is currently optional (until the native side is changed).

If an inner object has the request_id set, and the outer object does not, the outer object request ID is set to that inner one.

@benwtrent benwtrent requested a review from davidkyle July 12, 2022 16:38
@elasticmachine elasticmachine added the Team:ML Meta label for the ML team label Jul 12, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core (Team:ML)

Copy link
Member

@davidkyle davidkyle left a comment

Choose a reason for hiding this comment

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

LGTM

Please hold of merging for now in case we have to format again - the back end change isn't as simple as I imagined

@elasticsearchmachine elasticsearchmachine changed the base branch from master to main July 22, 2022 23:05
@mark-vieira mark-vieira added v8.5.0 and removed v8.4.0 labels Jul 27, 2022
@droberts195
Copy link

To make this most useful the cache hit indicator also needs to be on the top level.

I'd also prefer to make the Java and C++ changes in parallel and mute tests during the few hours where they're out-of-sync. It will make the code history cleaner in the future if anyone wants to look back at what was changed.

Since you're going to be on leave soon and this PR already has lots of merge conflicts I will close it and copy the relevant bits to a new PR in my fork, which can then get tested in combination with a C++ side PR in the C++ CI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:ml Machine learning >non-issue Team:ML Meta label for the ML team v8.5.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants