tillrohrmann commented on a change in pull request #6602: [FLINK-7551][rest]
Add versioning to REST API
URL: https://github.com/apache/flink/pull/6602#discussion_r213778724
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/rest/RestServerEndpoint.java
##########
@@ -364,22 +364,37 @@ public String getRestBaseUrl() {
}
}
- private static void registerHandler(Router router,
Tuple2<RestHandlerSpecification, ChannelInboundHandler> specificationHandler) {
- switch (specificationHandler.f0.getHttpMethod()) {
+ private static void registerHandler(Router router,
Tuple2<RestHandlerSpecification, ChannelInboundHandler> specificationHandler,
Logger log) {
+ final String handlerURL =
specificationHandler.f0.getTargetRestEndpointURL();
+ // setup versioned urls
+ for (final RestAPIVersion supportedVersion :
specificationHandler.f0.getSupportedAPIVersions()) {
+ final String versionedHandlerURL = '/' +
supportedVersion.getURLVersionPrefix() + handlerURL;
+ log.debug("Register handler {} under {}@{}.",
specificationHandler.f1, specificationHandler.f0.getHttpMethod(),
versionedHandlerURL);
+ registerHandler(router, versionedHandlerURL,
specificationHandler.f0.getHttpMethod(), specificationHandler.f1);
+ }
+ // setup unversioned url for convenience and backwards
compatibility
+ // this url will always point to the oldest supported version
+ // this is controlled by the order in which handler are
registered
+ log.debug("Register handler {} under {}@{}.",
specificationHandler.f1, specificationHandler.f0.getHttpMethod(), handlerURL);
+ registerHandler(router, handlerURL,
specificationHandler.f0.getHttpMethod(), specificationHandler.f1);
Review comment:
Ah ok, the idea is that the unversioned URLs point to the oldest supported
REST API version, right? The problem with this approach is that we would break
the API (unversioned URLs) every time we drop support for a given version.
Given that the versioned URLs are stable and we did not promise REST API
stability yet, this should most likely be ok.
The breaking change would only happen if users of the unversioned URLs don't
migrate to the versioned URLs and we drop support for a version. Once we do
this, we should add it to the migration guide.
Could we maybe add a check which only registers handlers of the oldest
version under the unversioned URLs instead of all?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
