opensearch-project / opensearch-rs

OpenSearch Rust Client
Apache License 2.0
63 stars 34 forks source link

updated compatibility information #189

Closed AbhinavGarg90 closed 1 year ago

AbhinavGarg90 commented 1 year ago

Description

updates comments regarding the compatibility policy to reflect that the client does not need to remain in major version lockstep with the server.

Issues Resolved

178

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 year ago

Codecov Report

Merging #189 (c14dcce) into main (94e7b72) will increase coverage by 0.00%. The diff coverage is n/a.

:exclamation: Current head c14dcce differs from pull request most recent head 597590c. Consider uploading reports for the commit 597590c to get more accurate results

@@           Coverage Diff           @@
##             main     #189   +/-   ##
=======================================
  Coverage   73.82%   73.82%           
=======================================
  Files         402      401    -1     
  Lines       63738    63736    -2     
=======================================
  Hits        47055    47055           
+ Misses      16683    16681    -2     
Flag Coverage Δ
integration 73.82% <ø> (+<0.01%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

see 2 files with indirect coverage changes

AbhinavGarg90 commented 1 year ago

I see tests in this PR run against OpenSearch 1.x and 2.x, so is this correct? AFAIK 2.x client is compatible with both OpenSearch 1.x and 2.x.

Could you please elaborate? What action is needed from my end?

Xtansia commented 1 year ago

I see tests in this PR run against OpenSearch 1.x and 2.x, so is this correct? AFAIK 2.x client is compatible with both OpenSearch 1.x and 2.x.

Mostly yes, though 2.x of the client dropped the "types" apis, so not 100% compatible with 1.x.

dblock commented 1 year ago

I see tests in this PR run against OpenSearch 1.x and 2.x, so is this correct? AFAIK 2.x client is compatible with both OpenSearch 1.x and 2.x.

Could you please elaborate? What action is needed from my end?

The line that says | 2.x | 2.x | is incorrect. It should be | 1.x - 2.x | 2.x | or something like that?

dblock commented 1 year ago

I see tests in this PR run against OpenSearch 1.x and 2.x, so is this correct? AFAIK 2.x client is compatible with both OpenSearch 1.x and 2.x.

Mostly yes, though 2.x of the client dropped the "types" apis, so not 100% compatible with 1.x.

I see. So do you think the code in this PR is correct? If so, then let's merge?

Xtansia commented 1 year ago

I see tests in this PR run against OpenSearch 1.x and 2.x, so is this correct? AFAIK 2.x client is compatible with both OpenSearch 1.x and 2.x.

Mostly yes, though 2.x of the client dropped the "types" apis, so not 100% compatible with 1.x.

I see. So do you think the code in this PR is correct? If so, then let's merge?

It's tricky as only listing 2.x as compatible definitely undersells the level of compatibility, but we also probably don't want to list 1.x without qualifying conditions in case we get complaints around missing functionality for 1.x.

But maybe we can do something like:


Rust client OpenSearch
1.x 1.x
2.x 2.x, 1.x^
AbhinavGarg90 commented 1 year ago

I see tests in this PR run against OpenSearch 1.x and 2.x, so is this correct? AFAIK 2.x client is compatible with both OpenSearch 1.x and 2.x.

Mostly yes, though 2.x of the client dropped the "types" apis, so not 100% compatible with 1.x.

I see. So do you think the code in this PR is correct? If so, then let's merge?

It's tricky as only listing 2.x as compatible definitely undersells the level of compatibility, but we also probably don't want to list 1.x without qualifying conditions in case we get complaints around missing functionality for 1.x.

But maybe we can do something like:

Rust client OpenSearch 1.x 1.x 2.x 2.x, 1.x^

  • ^: With the exception of some previously deprecated APIs

this sounds like a good idea. Gives a fair warning to somebody, but still ensure they know that it is compatible to an extent. Should I commit that change?

AbhinavGarg90 commented 1 year ago

the additional line has been added. Does that look fine @Xtansia ?