opensearch-project / opensearch-go

Go Client for OpenSearch
https://opensearch.org/docs/latest/clients/go/
Apache License 2.0
202 stars 106 forks source link

Return HTTP response as second return value from typed API client functions #624

Open ste93cry opened 1 month ago

ste93cry commented 1 month ago

Description

I'm refactoring the functions of the typed clients to return the HTTP response as second value.

Issues Resolved

Closes #619

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. For more information on following Developer Certificate of Origin and signing off your commits, please check here.

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 99.37107% with 7 lines in your changes missing coverage. Please review.

Project coverage is 56.70%. Comparing base (06a6dc8) to head (574ee61). Report is 70 commits behind head on main.

Files with missing lines Patch % Lines
opensearchapi/api_cat.go 95.00% 6 Missing :warning:
opensearchapi/api_cluster.go 98.61% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #624 +/- ## ========================================== - Coverage 57.29% 56.70% -0.59% ========================================== Files 315 374 +59 Lines 9823 8036 -1787 ========================================== - Hits 5628 4557 -1071 + Misses 2902 2171 -731 - Partials 1293 1308 +15 ``` | [Flag](https://app.codecov.io/gh/opensearch-project/opensearch-go/pull/624/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=opensearch-project) | Coverage Δ | | |---|---|---| | [integration](https://app.codecov.io/gh/opensearch-project/opensearch-go/pull/624/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=opensearch-project) | `56.70% <99.37%> (+5.86%)` | :arrow_up: | | [unit](https://app.codecov.io/gh/opensearch-project/opensearch-go/pull/624/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=opensearch-project) | `?` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=opensearch-project#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files with missing lines](https://app.codecov.io/gh/opensearch-project/opensearch-go/pull/624?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=opensearch-project) | Coverage Δ | | |---|---|---| | [opensearchapi/api\_aliases.go](https://app.codecov.io/gh/opensearch-project/opensearch-go/pull/624?src=pr&el=tree&filepath=opensearchapi%2Fapi_aliases.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=opensearch-project#diff-b3BlbnNlYXJjaGFwaS9hcGlfYWxpYXNlcy5nbw==) | `100.00% <100.00%> (ø)` | | | [opensearchapi/api\_bulk.go](https://app.codecov.io/gh/opensearch-project/opensearch-go/pull/624?src=pr&el=tree&filepath=opensearchapi%2Fapi_bulk.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=opensearch-project#diff-b3BlbnNlYXJjaGFwaS9hcGlfYnVsay5nbw==) | `100.00% <100.00%> (ø)` | | | [opensearchapi/api\_cat-aliases.go](https://app.codecov.io/gh/opensearch-project/opensearch-go/pull/624?src=pr&el=tree&filepath=opensearchapi%2Fapi_cat-aliases.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=opensearch-project#diff-b3BlbnNlYXJjaGFwaS9hcGlfY2F0LWFsaWFzZXMuZ28=) | `100.00% <ø> (ø)` | | | [opensearchapi/api\_cat-allocation.go](https://app.codecov.io/gh/opensearch-project/opensearch-go/pull/624?src=pr&el=tree&filepath=opensearchapi%2Fapi_cat-allocation.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=opensearch-project#diff-b3BlbnNlYXJjaGFwaS9hcGlfY2F0LWFsbG9jYXRpb24uZ28=) | `100.00% <ø> (ø)` | | | [opensearchapi/api\_cat-cluster\_manager.go](https://app.codecov.io/gh/opensearch-project/opensearch-go/pull/624?src=pr&el=tree&filepath=opensearchapi%2Fapi_cat-cluster_manager.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=opensearch-project#diff-b3BlbnNlYXJjaGFwaS9hcGlfY2F0LWNsdXN0ZXJfbWFuYWdlci5nbw==) | `100.00% <ø> (ø)` | | | [opensearchapi/api\_cat-count.go](https://app.codecov.io/gh/opensearch-project/opensearch-go/pull/624?src=pr&el=tree&filepath=opensearchapi%2Fapi_cat-count.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=opensearch-project#diff-b3BlbnNlYXJjaGFwaS9hcGlfY2F0LWNvdW50Lmdv) | `100.00% <ø> (ø)` | | | [opensearchapi/api\_cat-fielddata.go](https://app.codecov.io/gh/opensearch-project/opensearch-go/pull/624?src=pr&el=tree&filepath=opensearchapi%2Fapi_cat-fielddata.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=opensearch-project#diff-b3BlbnNlYXJjaGFwaS9hcGlfY2F0LWZpZWxkZGF0YS5nbw==) | `100.00% <ø> (ø)` | | | [opensearchapi/api\_cat-health.go](https://app.codecov.io/gh/opensearch-project/opensearch-go/pull/624?src=pr&el=tree&filepath=opensearchapi%2Fapi_cat-health.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=opensearch-project#diff-b3BlbnNlYXJjaGFwaS9hcGlfY2F0LWhlYWx0aC5nbw==) | `100.00% <ø> (ø)` | | | [opensearchapi/api\_cat-indices.go](https://app.codecov.io/gh/opensearch-project/opensearch-go/pull/624?src=pr&el=tree&filepath=opensearchapi%2Fapi_cat-indices.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=opensearch-project#diff-b3BlbnNlYXJjaGFwaS9hcGlfY2F0LWluZGljZXMuZ28=) | `100.00% <ø> (ø)` | | | [opensearchapi/api\_cat-master.go](https://app.codecov.io/gh/opensearch-project/opensearch-go/pull/624?src=pr&el=tree&filepath=opensearchapi%2Fapi_cat-master.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=opensearch-project#diff-b3BlbnNlYXJjaGFwaS9hcGlfY2F0LW1hc3Rlci5nbw==) | `100.00% <ø> (ø)` | | | ... and [197 more](https://app.codecov.io/gh/opensearch-project/opensearch-go/pull/624?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=opensearch-project) | | ... and [156 files with indirect coverage changes](https://app.codecov.io/gh/opensearch-project/opensearch-go/pull/624/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=opensearch-project)
dblock commented 1 month ago

Big breaking change :(

If data, response, err was flipped as data, err, response, would it be easier to migrate to? Does go allow to specify the first 2 objects and omit the third?

ste93cry commented 1 month ago

No, it doesn’t. Also, it's conventional in Go to return error as last value. But, even though it’s a breaking change, it’s quite easy to track it and solve it as the project will not compile until the new returned value is either ignored or handled, and find and replace in most cases might be all a user needs to do if he never bothers about the response. Considering that this change is targeting the next major version, where this kind of things is expected to happen, I don’t see big downsides.