-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Migrate org.elasticsearch.action to TransportVersion #93086
Conversation
Add initial set of javadocs
Add initial set of javadocs
TransportVersion tests
Pinging @elastic/es-core-infra (Team:Core/Infra) |
@@ -235,7 +235,7 @@ protected void doInternalExecute(Task task, BulkRequest bulkRequest, String exec | |||
} | |||
|
|||
if (actionRequest instanceof IndexRequest ir) { | |||
ir.checkAutoIdWithOpTypeCreateSupportedByVersion(minNodeVersion); | |||
ir.checkAutoIdWithOpTypeCreateSupportedByVersion(minNodeVersion.transportVersion); |
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.
minNodeVersion
will need to be migrated separately, its used in several other places
throw new IllegalArgumentException( | ||
"can't use sort by shard count or repository name with node version [" + out.getVersion() + "]" | ||
"can't use sort by shard count or repository name with version [" + out.getTransportVersion() + "]" |
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.
perhaps node->transport in the exception message would be useful?
out.writeVInt(offset); | ||
} else if (offset != 0) { | ||
throw new IllegalArgumentException( | ||
"can't use numeric offset in get snapshots request with node version [" + out.getVersion() + "]" | ||
"can't use numeric offset in get snapshots request with version [" + out.getTransportVersion() + "]" |
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.
node->transport version might be more clear?
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.
Updated all of these
@elasticsearchmachine rerun elasticsearch-ci/part-2 |
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
Migrate files in
org.elasticsearch.action
to use TransportVersion.Vast majority is
s/Version/TransportVersion/
, with a couple of comments