Introduce stability description to the REST API specification #38413

Mpdreamz · 2019-02-05T11:57:38Z

This introduces a state key to the REST API specs to signal an API's readiness:

state indicating the state of the API, defaults to stable
- private this API should not be be implemented by clients
- experimental highly likely to break in the near future (minor/path), no bwc guarantees.
  Possibly removed in the future.
- beta less likely to break or be removed but still reserve the right to do so
- stable No backwards breaking changes in a minor (default if not specified)

Mpdreamz · 2019-02-05T12:02:28Z

This could in the future also include e.g deprecated if we want to remove _source or other API's. Although no such plans currently exists.

Mpdreamz · 2019-02-05T13:45:44Z

For deprecations I prefer @mayya-sharipova proposal here: #35262

elasticmachine · 2019-02-06T00:07:43Z

Pinging @elastic/es-core-infra

Mpdreamz · 2019-02-13T15:27:16Z

Discussed during es-core-infra sync.

private should not be a state. If an API is considered private it should not be in the json rest spec.
- There are a couple of SAML endpoints undocumented as well that might warrant additional discussion (are they private?). How do we stay vigilant an API is not private by accident?
state property name is not a great name, either status or stability.
Ideally there is a test that enforces the new property only uses the enum values we settled on experimental, beta and stable, or empty defaulting to stable.

Mpdreamz · 2019-02-13T17:18:23Z

state property name is not a great name, either status or stability.

Addressed this item.

jkakavas · 2019-02-23T18:44:39Z

There are a couple of SAML endpoints undocumented as well that might warrant additional discussion (are they private?). How do we stay vigilant an API is not private by accident?

We want to document these two for some time. I'll open an issue for this so that folks can track process as we've been answering this question every once in a while, but I plan on tackling the docs in the next couple of weeks

Mpdreamz · 2019-03-04T14:41:40Z

Awesome to hear @jkakavas looking forward to those 👍

Mpdreamz · 2019-03-04T14:43:01Z

run elasticsearch-ci/2

jaymode

Overall, I think this is a good change. Ultimately I would like to see stability be a field that is required to be defined so that APIs do not get accidentally marked as stable due to leaving this field out. Additionally, there is at least one stable API that is not stable in terms of the response body, see #40104. Should we have a separate type for these?

jaymode · 2019-04-03T19:05:44Z

x-pack/plugin/src/test/resources/rest-api-spec/api/ccr.delete_auto_follow_pattern.json

@@ -1,6 +1,7 @@
 {
  "ccr.delete_auto_follow_pattern": {
    "documentation": "https://www.elastic.co/guide/en/elasticsearch/reference/current/ccr-delete-auto-follow-pattern.html",
+    "stability" : "beta",


I think all CCR APIs are GA #39722

ta, this PR preceeded that one. Updated now.

jaymode · 2019-04-03T19:09:46Z

x-pack/plugin/src/test/resources/rest-api-spec/api/sql.clear_cursor.json

@@ -1,6 +1,7 @@
 {
  "sql.clear_cursor": {
    "documentation": "Clear SQL cursor",
+    "stability" : "beta",


SQL APIs should be GA as of 6.7. @elastic/es-sql can you confirm?

Indeed, SQL is GA since 6.7. Thanks @jaymode for picking it up.

Updated, to reflect latest state changes. Thanks for confirming.

Mpdreamz · 2019-04-04T19:09:07Z

cc @elastic/es-clients thoughts on the effect of #40104 on this PR?

What should we call this type of API? its stable in terms of url/query_params but the body is reserved to be broken in between versions.

This API (GET /_cluster/state) is now for inspection purposes only and not an API users should be expected to write features against.

cc @elastic/microsoft #40104 has deep implications on how we map ClusterStateResponse in 7.9

russcam · 2019-04-04T20:49:00Z

It looks like this would be a candidate for the private state, or the REST spec for it ought to be removed

karmi · 2019-04-07T15:49:40Z

Here's a good example, in #39382 (emphasis mine):

This PR adds an internal REST API for querying context information about Painless whitelists.

The API specification doesn't include the documentation link even, so this one looks a very good candidate for private / internal API.

(Note that the client generators still need a documentation link.)

Mpdreamz · 2019-04-08T13:40:20Z

I opted for internal to represent the guarantees the cluster state API makes.

Its here to stay but we no longer make guarantees about its output staying the same during the course of a major version.

This is different then private IMO since these can be removed at any time during the lifetime of a major version.

jaymode · 2019-04-08T13:52:07Z

@DaveCTurner @jasontedor would love to get your input on the stability listed for the cluster state API and whether that matches your intentions.

DaveCTurner · 2019-04-08T14:53:20Z

Yes, internal sounds right to me amongst those choices. However I'd like to question whether we need to distinguish internal, beta and experimental at all. Although I see they have slightly different intentions, in practice I think they all really mean the same thing: "this API may change in incompatible ways in future, consume it at your own risk".

Mpdreamz · 2019-04-08T15:17:38Z

I would not want to oversimplify states too much:

beta expirimental signal features are in development towards stable but might be removed if we please.
private signals a client should not implement these at all, see e.g the data_frame json stubs introduced in #40283 on master
internal means this API is here to stay but the request/response do not follow normal bwc guarantees.

@jaymode I've amended the PR so that not specifying stability fails the yaml tests.

Also if someone specifies a stability that is not known the yaml tests will fail too stating the API which has the offending spec.

karmi · 2019-04-08T15:26:09Z

I agree with @Mpdreamz here — the distinction between experimental|beta, private and internal is important.

Mpdreamz · 2019-04-09T11:01:02Z

run elasticsearch-ci/2

jaymode

LGTM

…ility appears after documentation everywhere

Mpdreamz · 2019-04-24T08:07:27Z

run elasticsearch-ci/1

rjernst · 2019-04-24T21:37:46Z

The distinction is there because sometimes an API should be exposed in the client such as cluster.state.json with no backwards guarantees on the response which makes it internal.

I think this can be achieved in a different way. The api is "stable" in that the cluster state handler is not going away. It is the content that is unstable, ie the data returned by the cluster state api is an opaque blob.

The private was reintroduced seeing that sometimes stubs get added for documentation/development purposes e.g #40283.

I don't think those should have rest api specs at all. They can exist as http handlers in the code, and used eg for documentation generation, but not exist in the spec. That was the same conclusion we agreed on in the original discussion regarding private apis.

Mpdreamz · 2019-04-25T19:15:05Z

The semantics between internal and stable mean a lot to us clients.

It basically signals the difference between map these requests and responses or not.

internal to me means the response is an untyped blob/hash/map/dictionary of json but the API url and query parameters are stable and won't introduce breaking changes.

The progression of an API would be:

experimental => beta => internal or experimental => beta => stable

The meaning of internal is important here because experimental and beta API's will need to be delivered through add-on packages with their own versioning because many clients can not introduce breaking changes in minors.

private is now on:

data_frame.* API's
scripts_painless_context
monitoring.bulk

Given that we feel data_frame should have never been committed if they are stubs, and we are happy to role scripts_painless_context and monitoring.bulk under experimental I'd be happy to remove private.

Mpdreamz · 2019-04-26T10:02:40Z

An alternative option would be to mark

cluster.state as stable and mark the respose in the spec as response_blob: true

The dataframe.* specs were added to make them available in kibana during testing development which brings back private as a possible state or we ca rename private to internal(given we mark cluster.state as stable) which would make the progression:

internal => expirimental => beta => stable

With internal not being exposed at all, expirimental and beta being delivered out of band for clients that can not break in between major versions and stable going into the main packages.

rjernst · 2019-05-06T23:37:34Z

cluster.state as stable and mark the respose in the spec as response_blob: true

This is what I was suggesting, and I think it is simpler and more clear.

The dataframe.* specs were added to make them available in kibana during testing development which brings back private as a possible state

It is unclear how kibana can do testing against an api that is considered "private", as anything returned from it could possibly change at any time and break tests. I feel strongly these private or internal apis should simply not exist within our spec files, as the spec files by definition are the apis we expect users to use.

hendrikmuhs · 2019-05-09T10:24:59Z

@Mpdreamz

The data_frame.* API's should be beta

I know this hasn't been the case when you opened this PR, but meanwhile it progressed to beta (master and 7x)

…y value format that can be changed during minors

Mpdreamz · 2019-06-11T10:46:40Z

Addressed remaining PR comments:

data frame API's are now stable ty @hendrikmuhs 👍
cluster.state is now stable and the response is marked to be treated as key value like so

"response": {
  "treat_json_as_key_value" : true
}

remove internal and private as stability states.

There are 2 API's that are not really intended for public consumption though:

https://github.com/elastic/elasticsearch/commits/master/rest-api-spec/src/main/resources/rest-api-spec/api/scripts_painless_context.json (explicitly listed as internal on the PR: Add a Painless Context REST API #39382)
https://github.com/elastic/elasticsearch/blob/master/x-pack/plugin/src/test/resources/rest-api-spec/api/monitoring.bulk.json (no docs available, only used to send product monitoring info).

I wholeheartedly subscribe to not implementing private/internal API's and in effect blessing the java client as the defacto client e.g both of those API's are useful from other ecosystems too (beats/logstash/codesearch/apm et all). That said I changed these to be documented as experimental for now but that might not be the most accurate description of its intent either. Would love some more thoughts on how to address this @rjernst @elastic/es-clients and anyone else 👍

…ommit

rjernst · 2019-06-11T20:53:19Z

Thanks @Mpdreamz! I opened #43122 to move the painless spec file into the appropriate test, so it will no longer be found here by clients using the spec.

rjernst

LGTM other than the merge conflict due to the painless context api being moved.

…c#38413) * introduce state to the REST API specification * change state over to stability * CCR is no GA updated to stable * SQL is now GA so marked as stable * Introduce `internal` as state for API's, marks stable in terms of lifetime but unstable in terms of guarantees on its output format since it exposes internal representations * make setting a wrong stability value, or not setting it at all an error that causes the YAML test suite to fail * update spec files to be explicit about their stability state * Document the fact that stability needs to be defined Otherwise the YAML test runner will fail (with a nice exception message) * address check style violations * update rest spec unit tests to include stability * found one more test spec file not declaring stability, made sure stability appears after documentation everywhere * cluster.state is stable, mark response in some way to denote its a key value format that can be changed during minors * mark data frame API's as beta * remove internal and private as states for an API * removed the wrong enum values in the Stability Enum in the previous commit (cherry picked from commit 61c34bb)

#43278) * introduce state to the REST API specification * change state over to stability * CCR is no GA updated to stable * SQL is now GA so marked as stable * Introduce `internal` as state for API's, marks stable in terms of lifetime but unstable in terms of guarantees on its output format since it exposes internal representations * make setting a wrong stability value, or not setting it at all an error that causes the YAML test suite to fail * update spec files to be explicit about their stability state * Document the fact that stability needs to be defined Otherwise the YAML test runner will fail (with a nice exception message) * address check style violations * update rest spec unit tests to include stability * found one more test spec file not declaring stability, made sure stability appears after documentation everywhere * cluster.state is stable, mark response in some way to denote its a key value format that can be changed during minors * mark data frame API's as beta * remove internal and private as states for an API * removed the wrong enum values in the Stability Enum in the previous commit (cherry picked from commit 61c34bb)

#43279) * introduce state to the REST API specification * change state over to stability * CCR is no GA updated to stable * SQL is now GA so marked as stable * Introduce `internal` as state for API's, marks stable in terms of lifetime but unstable in terms of guarantees on its output format since it exposes internal representations * make setting a wrong stability value, or not setting it at all an error that causes the YAML test suite to fail * update spec files to be explicit about their stability state * Document the fact that stability needs to be defined Otherwise the YAML test runner will fail (with a nice exception message) * address check style violations * update rest spec unit tests to include stability * found one more test spec file not declaring stability, made sure stability appears after documentation everywhere * cluster.state is stable, mark response in some way to denote its a key value format that can be changed during minors * mark data frame API's as beta * remove internal and private as states for an API * removed the wrong enum values in the Stability Enum in the previous commit (cherry picked from commit 61c34bb)

Mpdreamz mentioned this pull request Feb 5, 2019

Deprecation process for the rest spec #38102

Closed

Mpdreamz mentioned this pull request Feb 5, 2019

Nest.Experimental package elastic/elasticsearch-net#3566

Closed

jtibshirani added the :Core/Infra/REST API REST infrastructure and utilities label Feb 6, 2019

jtibshirani added the >enhancement label Feb 6, 2019

danielmitterdorfer added team-discuss and removed team-discuss labels Feb 13, 2019

Mpdreamz force-pushed the spec-api-status branch from 2d6b084 to cc855a3 Compare February 18, 2019 11:50

jaymode self-requested a review March 27, 2019 15:38

jaymode requested changes Apr 3, 2019

View reviewed changes

Mpdreamz force-pushed the spec-api-status branch from cc855a3 to bd29ddc Compare April 8, 2019 13:27

Mpdreamz force-pushed the spec-api-status branch from 0428aef to e9cfcb0 Compare April 9, 2019 13:10

jaymode approved these changes Apr 22, 2019

View reviewed changes

introduce state to the REST API specification

d08f75d

found one more test spec file not declaring stability, made sure stab…

130a75a

…ility appears after documentation everywhere

codebrain changed the title ~~introduce state to the REST API specification~~ Introduce stability description to the REST API specification May 24, 2019

Mpdreamz added 4 commits June 11, 2019 11:49

Merge branch 'master' into spec-api-status

21d46f4

cluster.state is stable, mark response in some way to denote its a ke…

1aaa327

…y value format that can be changed during minors

mark data frame API's as beta

73dea92

remove internal and private as states for an API

0c0f6ba

removed the wrong enum values in the Stability Enum in the previous c…

2437628

…ommit

Mpdreamz mentioned this pull request Jun 12, 2019

Treat cluster state as key value response in 7.0 elastic/elasticsearch-net#3812

Closed

rjernst approved these changes Jun 12, 2019

View reviewed changes

Merge branch 'master' into spec-api-status

e3a2fad

Mpdreamz merged commit 61c34bb into elastic:master Jun 12, 2019

pcsanwald added the v8.0.0 label Jun 14, 2019

Mpdreamz mentioned this pull request Jun 17, 2019

Feature/7.x/cluster state dynamic elastic/elasticsearch-net#3824

Merged

lukas-vlcek mentioned this pull request Jul 12, 2019

Elastic v7.2.0 is now available vvanholl/elasticsearch-prometheus-exporter#198

Closed

Mpdreamz mentioned this pull request Apr 15, 2020

Rest spec and documentation #54664

Merged

Mpdreamz mentioned this pull request May 4, 2020

Add visibility the to rest-spec-api #56104

Merged

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce stability description to the REST API specification #38413

Introduce stability description to the REST API specification #38413

Mpdreamz commented Feb 5, 2019

Mpdreamz commented Feb 5, 2019 •

edited

Loading

Mpdreamz commented Feb 5, 2019

elasticmachine commented Feb 6, 2019

Mpdreamz commented Feb 13, 2019

Mpdreamz commented Feb 13, 2019

jkakavas commented Feb 23, 2019

Mpdreamz commented Mar 4, 2019

Mpdreamz commented Mar 4, 2019

jaymode left a comment

jaymode Apr 3, 2019

Mpdreamz Apr 8, 2019

jaymode Apr 3, 2019

costin Apr 7, 2019

Mpdreamz Apr 8, 2019

Mpdreamz commented Apr 4, 2019

russcam commented Apr 4, 2019

karmi commented Apr 7, 2019

Mpdreamz commented Apr 8, 2019

jaymode commented Apr 8, 2019

DaveCTurner commented Apr 8, 2019

Mpdreamz commented Apr 8, 2019

karmi commented Apr 8, 2019

Mpdreamz commented Apr 9, 2019

jaymode left a comment

Mpdreamz commented Apr 24, 2019

rjernst commented Apr 24, 2019

Mpdreamz commented Apr 25, 2019

Mpdreamz commented Apr 26, 2019

rjernst commented May 6, 2019

hendrikmuhs commented May 9, 2019

Mpdreamz commented Jun 11, 2019

rjernst commented Jun 11, 2019

rjernst left a comment

Introduce stability description to the REST API specification #38413

Introduce stability description to the REST API specification #38413

Conversation

Mpdreamz commented Feb 5, 2019

Mpdreamz commented Feb 5, 2019 • edited Loading

Mpdreamz commented Feb 5, 2019

elasticmachine commented Feb 6, 2019

Mpdreamz commented Feb 13, 2019

Mpdreamz commented Feb 13, 2019

jkakavas commented Feb 23, 2019

Mpdreamz commented Mar 4, 2019

Mpdreamz commented Mar 4, 2019

jaymode left a comment

Choose a reason for hiding this comment

jaymode Apr 3, 2019

Choose a reason for hiding this comment

Mpdreamz Apr 8, 2019

Choose a reason for hiding this comment

jaymode Apr 3, 2019

Choose a reason for hiding this comment

costin Apr 7, 2019

Choose a reason for hiding this comment

Mpdreamz Apr 8, 2019

Choose a reason for hiding this comment

Mpdreamz commented Apr 4, 2019

russcam commented Apr 4, 2019

karmi commented Apr 7, 2019

Mpdreamz commented Apr 8, 2019

jaymode commented Apr 8, 2019

DaveCTurner commented Apr 8, 2019

Mpdreamz commented Apr 8, 2019

karmi commented Apr 8, 2019

Mpdreamz commented Apr 9, 2019

jaymode left a comment

Choose a reason for hiding this comment

Mpdreamz commented Apr 24, 2019

rjernst commented Apr 24, 2019

Mpdreamz commented Apr 25, 2019

Mpdreamz commented Apr 26, 2019

rjernst commented May 6, 2019

hendrikmuhs commented May 9, 2019

Mpdreamz commented Jun 11, 2019

rjernst commented Jun 11, 2019

rjernst left a comment

Choose a reason for hiding this comment

Mpdreamz commented Feb 5, 2019 •

edited

Loading