Introduce Admin API version and verify by CLI #1538
Conversation
|
I'm not entirely sure how this will work. We have control over the server but not the CLI, I'd have imagined the flow to be the opposite. The CLI sends its own version (what we print in Are you suggesting that the server adjusts its protocol/interface based on a negotiated version (similar to how we do with SDKs)? wouldn't that be too much work, given how easy it is for users to update the CLI? |
| @@ -80,7 +80,7 @@ pub struct GlobalOpts { | |||
| pub enum Command { | |||
| /// Prints general information about the configured environment | |||
| #[clap(name = "whoami")] | |||
| WhoAmiI(whoami::WhoAmI), | |||
| WhoAmI(whoami::WhoAmI), | |||
The idea of this PR is that the server offers a set of versioned admin APIs which the client can choose from if it knows one of them. How different versions of the admin API are distinguished/used is left open to the specific version (e.g. via a different path or some special header). I expect the CLI to support a single admin API version. Depending on how we evolve the admin API, servers might support for some time multiple versions (e.g. until an older version is deprecated). For the sake of simplicity, I've opted for a coarse-grained versioning of the API (deeming the whole CLI incompatible even if some of the endpoints would still be compatible). Additionally, I thought that a partially functioning CLI might be surprising for users. Moving the version check into the server should work as well, I believe. Sending the semver version of the CLI and not the admin API version can lead to a problem if a newer CLI asks the server for a compatibility check, I believe. In this case, the server won't be able to answer whether the CLI is compatible or not (the only viable option would be to declare it as incompatible and ask for a downgrade).
No, I think this would not be worth it. |
| @@ -176,7 +176,7 @@ pub async fn run(State(env): State<CliEnv>) { | |||
|
|
|||
| c_println!(); | |||
| // Get admin client, don't fail completely if we can't get one! | |||
| if let Ok(client) = crate::clients::MetasClient::new(&env) { | |||
| if let Ok(client) = crate::clients::AdminClient::new(&env).await { | |||
There was a problem hiding this comment.
Do you think we should print the supported version in whoami's output?
There was a problem hiding this comment.
It probably wouldn't hurt. Will add it.
With this commit, the former client uses the latter to share common setup logic.
This commit introduces the Admin API version and exposes it via the /version REST endpoint. The Admin API version is used by the CLI to verify that it is compatible with the Admin server. Ideally, we evolve the Admin API in a backwards compatible way so that we never need to bump the Admin API version. However, in case this is not possible, we can now introduce a new version and later deprecate older versions. Old CLI that only support removed Admin API versions will learn about it and tell their users to upgrade the CLI. This fixes restatedev#1489.
|
Thanks for the review @AhmedSoliman. Merging this PR once GHA gives green light. |
This commit introduces the Admin API version and exposes it via the
/version REST endpoint. The Admin API version is used by the CLI to
verify that it is compatible with the Admin server. Ideally, we evolve
the Admin API in a backwards compatible way so that we never need to
bump the Admin API version. However, in case this is not possible, we
can now introduce a new version and later deprecate older versions.
Old CLI that only support removed Admin API versions will learn about
it and tell their users to upgrade the CLI.
This fixes #1489.