search

elastic / elasticsearch

Free and Open Source, Distributed, RESTful Search Engine
https://www.elastic.co/products/elasticsearch
Other
1.1k stars 24.83k forks source link

Ensure old rest api compatible code is not used after version bump #70617

Open pgomulka opened 3 years ago

pgomulka commented 3 years ago

Rest API compatibility is only supporting N-1 version. We want to ensure that after major version bump we won't break the build and the N-2 code won't be used by the server. During the major version bump we expect to remove old Version fields and bump the RestApiVersion.CURRENT

Currently we emit an assertion error in https://github.com/elastic/elasticsearch/blob/master/server/src/main/java/org/elasticsearch/rest/RestController.java#L179 which would break the build after the major version bump is performed and N-2 compatible code is not removed. 

        assert RestApiVersion.minimumSupported() == version || RestApiVersion.current() == version
            : "REST API compatibility is only supported for version " + RestApiVersion.minimumSupported().major;

For instance Version.CURRENT = 9.0.0, RestApiVersion.current = 9 and RestApiVersion.minimumSupported = 8. The old routes in RestActions are still using RestApiVersion.V_7

The same applies to testing. Unit tests that are covering individual RestAction with old routes, will fail on those assertions.

Possible solution would be to remove the assertion in RestController and emit a warning/error in logs

relates https://github.com/elastic/elasticsearch/issues/51816 follow up after https://github.com/elastic/elasticsearch/pull/69131#discussion_r596911219

elasticmachine commented 3 years ago

Pinging @elastic/es-core-infra (Team:Core/Infra)

pgomulka commented 3 years ago

we discussed this on compatible rest api sync on Mon 22th March. We concluded that we don't want to throw an error when an old code is still present and accidentally registered. The goal will be to not to register it. The registration of the old rest handlers will be ignored, possibly with a special logging level/trace/debug. We will also work on a gradle task to count references and have a quick way of finding how much more compatible code is to be removed. The testing gradle task yamlRestCompatTest#v7 (or other version) will have to be removed at the same time as the version bump is performed. This will have to be added into a documentation, or worked around with a dummy v7 task.