Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiversion Support #124

Open
koarlchen opened this issue Jan 5, 2025 · 2 comments
Open

Multiversion Support #124

koarlchen opened this issue Jan 5, 2025 · 2 comments

Comments

@koarlchen
Copy link

koarlchen commented Jan 5, 2025

Hi, so I experimented a bit on how to handle different version in a clean way. The solution I came up with would require changes to the SpaceAPI schema. So I'm not sure if this issue is the right place for it ;)

Proposal

The only thing I came up with is apparently not compatible with the current specification. We would have to add a field like version that states the exact version of the currently provided api. Together with that change I would suggest to make the api_compatibility field optional. IIRC, the version field was removed with schema v14.
Having a distinct version field was also the outcome of a short google-research on how to implement/handle different API versions (not only in Rust, but in a more general approach).

Assuming we have the version field, we could do something like this:

use serde::{Serialize, Deserialize};

#[derive(Serialize, Deserialize, Debug)]
#[serde(tag = "version")]
enum Status {
    V14(StatusV14),
    V15(StatusV15),
}

#[derive(Serialize, Deserialize, Debug)]
struct StatusV14 {
    pub field_a: u32,
}

#[derive(Serialize, Deserialize, Debug)]
struct StatusV15 {
    pub field_a: u32,
    pub field_b: u32,
}

fn main() {
    let json_14 = "{ \"version\": \"V14\", \"field_a\": 1 }";
    let json_15 = "{ \"version\": \"V15\", \"field_a\": 2, \"field_b\": 3 }";

    let status: Status = serde_json::from_str(&json_15).unwrap();
    println!("{:?}", status);

    let raw = serde_json::to_string(&status).unwrap();
    println!("{}", raw);
}

FYI, this implementation or rather the enum reminds me of the type std::net::IpAddr (see here)

Don't get me wrong, I do not intend to change the schema solely for making the implementation in Rust more straight forward. In my personal opinion, the server of the SpaceAPI should just serve one specific version mentioned in one of its json fields. The server could also provide multiple endpoints serving different versions. The client side, that queries that data should take care of the version stuff and should decide on which version it supports. So to speak, the person that implements the client side should take care of checking, which versions are compatible or rather which fields of interest are available in which versions and act accordingly. In essence, I suggest to move the compatiblity decision from the server to the client side.

Is that something you can agree on?

@dbrgn
Copy link
Collaborator

dbrgn commented Jan 5, 2025

Making it possible to be compatible with multiple versions at once was explicitly the goal of introducing the api_compatibility field, so this is unlikely to be changed 🙂

My suggestion in #118 (comment) would be to keep supporting 2 versions at once, meaning that v0.13 support is removed when adding v15. This way we can work with manually designed builders and validation logic, without having to rely on Serde for that.

@rnestler
Copy link
Member

My proposal would be to have a builder API like this:

pub struct StatusBuilder {
    versions: Vec<ApiVersion>,
    space: String,
    ...
}
impl StatusBuilder {
    StatusBuilder::new(space_name: S, api_versions: Vec<ApiVersion>) -> StatusBuilder
}

(We could make it a not owned version with lifetimes, but I do not really care, since it's only allocated once).

All the verify functions also take the versions slice and check accordingly.

That way it's very easy to support future versions very easily.

When de-serializing I'd just accept everything what serde allows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants