[GitHub] flink pull request #4623: [FLINK-7551] [REST] Add versioning

[zentol <gi...@git.apache.org>]

GitHub user zentol opened a pull request:

    https://github.com/apache/flink/pull/4623

     [FLINK-7551] [REST] Add versioning

    ## What is the purpose of the change
    
    This PR adds versioning to the REST API(s).
    
    The `RestAPIVersion` enum contains a view over all available rest api versions. A version represents the state of **all** REST API components, meaning that if any component does an incompatible change to its API the global API version has to be increased (which should happen at most once per release).
    While it would in principal be desirable to have separate versions per component this is effectively not possible since the communication protocols (i.e the parts where incompatible changes were made) are defined by MessageHeaders, MessageParameters and Requests-/ResponseBodies classes which are not tied to a specific server and re-usable by any server implementation. There are also other issues, like how to define version for rest servers that handle multiple components (like dispatcher + webmonitor).
    
    Whether a request is supported by a given version is defined by the MessageHeaders, that return a set of supported version via `abstract MessageHeaders#getSupportedAPIVersion()`.
    
    The client & server inject the api version as a path parameter `/:version` at the beginning of the URl.
    
    Unless explicitly specified the `RestClient` will always pick the latest supported version for the given header.
    
    On the server-side, for every request the `AbstractRestHandler` compares the request version aggainst the set of supported versions of the handler, and reject if with a 404 if it isn't supported. It returns a 404 since the fact that any handler received a message for an unsupported URL is a side-effect of the URL parameterization made by the Router. If we wouldn't use this feature we would instead hard-code the URLs for each supported version, and thus for unsupported versions a 404 would be returned since no handler was registered for that URL.
    
    ### Required changes for any version increment
    
    Incrementing the global API version requires the addition of a new constant to the `RestAPIVersion` enum, whose version number must be higher than any other. Furthermore, all existing `MessageHeaders` classes that are not affected by incompatible changes must be modified to contain the new version.
    
    If an incompatible change was made to a `MessageHeaders` class a new version of the class must be created. The same principle applies to MessageParameters and Request-/ResponseBodies. This will also require a new Handler implementation (due to generic parameters), that may share code with the previous handler implementation.
    
    ### How to handle multiple versions in a handler
    
    Handlers may retrieve the version of the request like any path parameter from the `HandlerRequest`. The
    corresponding parameter class is `VersionPathParameter`. Handlers that extend `AbstractRestHandler` are guaranteed to only be called if the version is supported as defined by the message headers.
    
    Usages of this parameter should be restricted for ensuring compatibility with a previous version with explicit special cases. Special cases must always refer to older versions, and **never latest version**.
    
    That is, provided version V1 and V2 exist,
    this code is fine:
    ```
    if (version == v1) {
    <ensure backwards compatibility>
    }
    ```
    
    where as this code is horrible since it is bound to be broken by a version increment:
    ```
    if (version != v2) {
    <ensure backwards compatibility>
    }
    OR
    if (version == V2) {
    ...
    } else {
    <ensure backwards compatibility>
    }
    ```
    
    ## Brief change log
    
    * add logging to RouterHandler (came up during testing)
    * Refactor RestEndpointITCase to make extensions easier (and the diff of the next commit cleaner)
    * Add `RestAPIVersion` enum and integrate it into headers, client and server
    * add implicit `VersionPathParameter` to all `MessageParameters`; effectively the version parameter is injected when calling `MessageParameters#getPathParameters`
    
    ## Verifying this change
    
    This change added tests and can be verified as follows:
    
    * RestEndpointITCase
      * verifies that versions are properly added to the URL
      * verifies that specific versions may be targeted through the RestClient
      * verifies that invalid versions are reject
    
    ## Does this pull request potentially affect one of the following parts:
    
      - Dependencies (does it add or upgrade a dependency): (no)
      - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (yes, but the API is unreleased)
      - The serializers: (no)
      - The runtime per-record code paths (performance sensitive): (no)
      - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no)
    
    ## Documentation
    
      - Does this pull request introduce a new feature? (no)
      - If yes, how is the feature documented? (not applicable)
    


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/zentol/flink 7551

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/flink/pull/4623.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #4623
    
----
commit 0b854fc9049e86de1de3d5955d2c2939f6c6b28e
Author: zentol <ch...@apache.org>
Date:   2017-08-30T10:20:08Z

    [hotfix] [REST] Add logging to RouterHandler

commit c96e08ca707a044b83c2618b10ebbceb055c6072
Author: zentol <ch...@apache.org>
Date:   2017-08-30T10:27:57Z

    [hotfix] [REST] Refactor RestEndpointITCase

commit a83b87fcf71abab0d0d07e96ccd7265ebd6cda93
Author: zentol <ch...@apache.org>
Date:   2017-08-29T15:30:26Z

    [FLINK-7551] [REST] Add versioning

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #4623: [FLINK-7551] [REST] Add versioning

[zentol <gi...@git.apache.org>]

Github user zentol closed the pull request at:

    https://github.com/apache/flink/pull/4623


---