Remove "force" version type #47228
Remove "force" version type #47228
Conversation
It's been deprecated long ago and can be removed.
ywelsch
added
>non-issue
:Distributed Indexing/CRUD
|
Pinging @elastic/es-distributed |
|
Note cc @elastic/es-clients This is a good highlight for the API versioning discussion, ideally this is only removed in major versions. cc @jakelandis |
Mark as obsolete to warn the user but without breaking backwards compatibily. The 7.5 client should be able to be used against < 7.5 server. relates: github.com/elastic/elasticsearch/pull/47228
* update specs * update ignore list * sync docgen changes from master, ignore time enum for now since its values are descriptions and we already have a Time type * codegen line ending changes * Add back local/master_timeout And document them as deprecated relates: elastic/elasticsearch#47933 * do not generate time and byte querystring parameters * patch now also hoists params over, we can no longer rely on simple merge since we temporarily still rework the spec files in memory to the old format * add changes and remove bytes for now * Add back force as option for version_type Mark as obsolete to warn the user but without breaking backwards compatibily. The 7.5 client should be able to be used against < 7.5 server. relates: github.com/elastic/elasticsearch/pull/47228 * codegen for enums can now pragma disable obsolete members in generated GetString() * Generated pragma disable for obsolete enum members
As per elastic#47228 `force` is no longer a valid option
@Mpdreamz note that this parameter had been rejected already since the previous major. I consider this purely a code clean-up. The only behavioral change here is that a more generic exception message is provided if someone specifies force version type. The question here rather is: When should the REST spec have been adapted? If the system always rejects a given parameter, is there a point to keep that parameter in the REST spec? |
The REST API spec should have been adapted when the version type was deprecated in the server source code; if deprecation results in a validation error, this sounds like good cause to remove from the spec if the deprecation is for a major release, or to be signalled in the REST API spec as deprecated for any other release.
A lot of the Elasticsearch clients generate a large amount of their API surface from the REST API specs, including API params. For those params that constrain the accepted values to a known set, a client may generate code to uphold this constraint, such as an enum of values. When the REST API spec changes in ways that would violate semantic versioning e.g. removing params in a minor version, removing an accepted value for a param in a minor, etc., it has ramifications for client generation; for statically typed languages like Java1, C#, Go and Rust, such a change in the REST API spec can manifest as an API breaking change in the client. Since clients also follow semantic versioning2 in order to provide guarantees to their consumers, API breaking changes should not happen in minors or patches, and if they do, it results in each client having to independently patch their code generation process to deal with them. The REST API specs are currently the best specification for the REST API surface of Elasticsearch that we have to work with, and the best representation of the external contract that clients, products and integrations use to communicate with Elasticsearch. I think it's crucially important that it follows the same semantic versioning rigour that I know the Elasticsearch team puts into the public API surface of the server side code. 1 - I don't know how the Java client is written, whether it generates the API surface from the REST API specs, and to what extent the API is typed |
|
Thanks for the explanations @russcam. If this is critical for client development, could we put safeguards in place that check API compatibility with prior versions of the spec, to avoid problems like this in the future? |
|
Safeguards would be very welcome! It could start with something more simple such as a diff of *.json files in a PR for changed files in
and flag when a change is breaking between two known versions. There is an initiative to record requests and responses of YAML tests to notify of changes in structure which should also help, but because it is detached from server side changes, it's more "after the fact" rather than flagging when a change will be merged into a main branch. |
As per #47228 `force` is no longer a valid option
It's been deprecated long ago and can be removed.
Relates to #20377
Closes #19769