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

Add missing codegen_name annotations to all value bodied responses #2457

Merged
merged 3 commits into from
Mar 22, 2024

Conversation

flobernd
Copy link
Member

@flobernd flobernd commented Mar 14, 2024

In the .NET client, these value-type bodies will be wrapped in a common response type like e.g.:

public partial class EsqlQueryResponse : 
    ElasticsearchResponse
{
    public byte[] Data { init; }
}

In order to properly auto-generate the required body property (e.g. Data in this case) with a meaningful name, all of these bodies must be annotated with codegen_name attributes.

It would be nice having a compiler check for this rule in the future.

@flobernd flobernd requested a review from a team March 14, 2024 12:06
Copy link
Member

@swallez swallez left a comment

Choose a reason for hiding this comment

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

Let me check if this causes breaking changes on the Java client...

Copy link
Member

@swallez swallez left a comment

Choose a reason for hiding this comment

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

LGTM, but I'd like @Anaethelion to also have a look from the Go client perspective.

@swallez swallez requested a review from Anaethelion March 14, 2024 15:49
Copy link
Contributor

@Anaethelion Anaethelion left a comment

Choose a reason for hiding this comment

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

LGTM, no impact on my part.

@flobernd flobernd requested review from a team as code owners March 22, 2024 14:54
Copy link
Contributor

Following you can find the validation results for the APIs you have changed.

API Status Request Response
cluster.remote_info 🟢 3/3 3/3
esql.query 🔴 156/162 7/162
get_source 🟢 18/18 18/18
ilm.get_lifecycle 🟢 8/8 8/8
indices.disk_usage 🟢 5/5 5/5
indices.downsample 🟢 1/1 1/1
indices.get_alias 🔴 83/83 71/83
indices.get_field_mapping 🔴 15/15 14/15
indices.get_mapping 🔴 91/91 87/91
indices.get_settings 🔴 56/56 54/56
indices.get_template 🟢 30/30 30/30
indices.get 🟢 53/53 53/53
indices.promote_data_stream Missing test Missing test
indices.recovery 🟢 9/9 9/9
indices.resolve_cluster 🔴 10/12 12/12
ingest.get_pipeline 🔴 22/22 21/22
logstash.get_pipeline Missing test Missing test
ml.preview_datafeed 🔴 10/17 17/17
rollup.get_rollup_caps 🟢 3/3 3/3
rollup.get_rollup_index_caps 🟢 7/7 7/7
search_application.get_behavioral_analytics Missing test Missing test
search_mvt 🔴 34/34 0/34
searchable_snapshots.clear_cache 🟢 1/1 1/1
security.delete_privileges 🟢 6/6 6/6
security.get_privileges 🟢 12/12 12/12
security.get_role_mapping 🔴 18/18 10/18
security.get_role 🟢 20/20 20/20
security.get_service_accounts Missing test Missing test
security.get_user 🟢 25/25 25/25
security.put_privileges 🟢 10/10 10/10
slm.get_lifecycle 🟢 16/16 16/16

You can validate these APIs yourself by using the make validate target.

@flobernd flobernd merged commit b3539b8 into main Mar 22, 2024
6 checks passed
@flobernd flobernd deleted the response-body-codegen branch March 22, 2024 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants