Skip to content

Add API Version to JellyfinClient #45

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

Merged
merged 4 commits into from
May 12, 2025
Merged

Conversation

JPKribs
Copy link
Member

@JPKribs JPKribs commented May 9, 2025

Summary

Let me know if there is a better way to do this. This finds the Jellyfin Version in the OpenAPI spec and writes them to an extension of JellyfinClient. These are both accessible like this:

JellyfinClient.sdkGeneratedVersion
JellyfinClient.sdkMinimumVersion

Purpose

The goal on the SF side is to use this to track the API minimum version so we can alert users who are on older version for compatibility checks.

Why Are Other Files Updated??

It looks like some of the comments in the updated between 10.10.6 and 10.10.7. I have no issue just using 10.10.6 but I also didn't think there was any harm running this on latest with this change.

Side Note

I went down the rabbit hole of like "Hey, if I'm doing this I should try to use swift-openapi-generator as well!" and I lost like 3 days trying that... It looks like we'd only be able to import Clients, Types, or Server instead of Enum, Entities, Path so it felt like it was a bust unless we want to build out a ton of post processing. I had a pretty bad time working on it so I'm not eager to look at this again haha.

@JPKribs JPKribs requested a review from LePips May 9, 2025 04:57
Copy link
Member

@LePips LePips left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting server/meta details is something I've wanted from CreateAPI a long time ago: CreateAPI/CreateAPI#147

We wouldn't necessarily want the version command to be decoupled from the generation. I would be more comfortable if this was in Swift alongside the generation:

private func generateVersionFile(context: PluginContext) async throws {
    let contents = try await parseOriginalSchema(context: context)
    
    guard case let .object(file) = contents else { return }
    guard case let .object(info) = file["info"] else { return }
    guard let version = info["version"]?.value as? String else { return }
    
    // modify file
}

@JPKribs
Copy link
Member Author

JPKribs commented May 10, 2025

would be more comfortable if this was in Swift alongside the generation

I've done that in my latest commit. Please let me know if this is what you meant. This would still be the same call to get this information:

JellyfinClient.sdkGeneratedVersion
JellyfinClient.sdkMinimumVersion

I put this in a JellyfinClient Extension because it's kind of the "client's version" but I'm game to change this to wherever makes sense to you. Also, not totally sold on those names. They feel a little long but I went descriptive JIC.

@LePips
Copy link
Member

LePips commented May 12, 2025

I've added a JellyfinClient.Version type which represents a semantic version and can be compared when we do it in the client. The "minimum SDK version" would mostly only be for major version server updates where API breakages can happen. All other checks can just be made against the major and minor version between the client and the generated SDK version.

@LePips LePips merged commit 6039de8 into jellyfin:main May 12, 2025
@JPKribs
Copy link
Member Author

JPKribs commented May 12, 2025

That's a good change! I was thinking about a parse func as well since I think the SystemInfo and PublicSystemInfo both return as a string so comparison requires some parsing. What you did looks great IMO

Thank you for the changes!

@JPKribs
Copy link
Member Author

JPKribs commented Jun 14, 2025

@LePips when you have a chance would you be able to make a 0.5.2 release with this included? I was going to start looking at a version warning again to start getting ahead of SF 1.3 version issues like:

jellyfin/Swiftfin#1570

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

Successfully merging this pull request may close these issues.

2 participants