Skip to content
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

Merged
merged 31 commits into from
Jan 26, 2023

Conversation

thecoop
Copy link
Member

@thecoop thecoop commented Jan 19, 2023

Migrate files in org.elasticsearch.action to use TransportVersion.

Vast majority is s/Version/TransportVersion/, with a couple of comments

@thecoop thecoop marked this pull request as ready for review January 24, 2023 10:22
@elasticsearchmachine elasticsearchmachine added the Team:Core/Infra Meta label for core/infra team label Jan 24, 2023
@elasticsearchmachine
Copy link
Collaborator

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);
Copy link
Member Author

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() + "]"
Copy link
Contributor

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() + "]"
Copy link
Contributor

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated all of these

@thecoop
Copy link
Member Author

thecoop commented Jan 26, 2023

@elasticsearchmachine rerun elasticsearch-ci/part-2

Copy link
Contributor

@pgomulka pgomulka left a comment

Choose a reason for hiding this comment

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

LGTM

@thecoop thecoop merged commit cfb2e4d into elastic:main Jan 26, 2023
@thecoop thecoop deleted the transportversion-actions branch January 26, 2023 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Transport API Transport client API >refactoring Team:Core/Infra Meta label for core/infra team v8.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants