[
https://issues.apache.org/jira/browse/FLINK-7551?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16596674#comment-16596674
]
ASF GitHub Bot commented on FLINK-7551:
---------------------------------------
zentol 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_r213781696
##########
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:
I just realized that the current approach in the PR has a troublesome flaw:
the default API is actually mixed iff a newer version defines a new url. (if we
add `/totally/new/url` in `v2` then the default for other urls would be `v1`,
but for the new one it would be `v2`).
As you suggested we should restrict the default to a specific version, which
would also solve the issue of multiple registrations.
What we could do is add a `isDefault` flag to the `RestAPIVersion` enum, and
only register unversioned urls for that version. We could enforce with a test
that exactly one version is marked as the default. WDYT?
----------------------------------------------------------------
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
> Add VERSION to the REST urls.
> ------------------------------
>
> Key: FLINK-7551
> URL: https://issues.apache.org/jira/browse/FLINK-7551
> Project: Flink
> Issue Type: Improvement
> Components: REST
> Affects Versions: 1.4.0
> Reporter: Kostas Kloudas
> Assignee: Chesnay Schepler
> Priority: Critical
> Labels: pull-request-available
> Fix For: 1.7.0
>
>
> This is to guarantee that we can update the REST API without breaking
> existing third-party clients.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
