-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[jellyfin] Add support for server versions > 10.8 #17757
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Patrik Gfeller <[email protected]>
Signed-off-by: Patrik Gfeller <[email protected]>
409fd45
to
9308073
Compare
@openhab/add-ons-maintainers Hello 🙂, I'm evaluating the possibility to use a code generator to simplify integration of multiple API versions to support legacy and current Jellyfin servers. This might also simplify to follow future API changes without the need to use the official SDK which mainly targets Android/Kotin and introduces some unwanted dependencies ... I checked the coding guidelines; and there seems to be no official place in the structure to include such tools; like scripts and code that is used to generate code. How is that handled - I'm not the maintainer of this binding; but would like make the tools (after proper review) available in case I'm hit by a 🚍 I propose to preserve the code somewhere. Is there already an established way to do this; and if not - is it worth to discuss ➡️ then I would open a discussion outside of this PR to find a consensus. Evaluation is not yet complete - but it is likely, that if I'm updating the API to support multiple versions some kind of code generation is used. |
Maybe look at how the miio binding has a ‘plugin’ that generates the documentation. I think your proposal is very similar. |
Thank you for the hint! I need to have a deeper look into the code, but at first glance it looks as m The difference to Jellyfin is, that the API and DTOs did change between the versions - therefore the DTOs did change as well as the endpoints. In theory the OpenAPI information could be used to generate the whole communication layer on the fly - but that code would be difficult to maintain and provide similar functionality to the already existing generators. @marcelrv I could avoid dedicated scripts to create/update the API code by including the step into the maven build step - but as I tend towards the OpenAPI Generator - not Swagger the following issue will prevent this: I'm also a bit hesitant to do this - but that's a discussion for later. For a first proof of concept I can generate the layer manually. In case we go that road the issue in the generator is something we could potentially fix. First let's see if my interpretation of the miio code is correct; and if not, if I'm able to generate a functional API layer using the openapi.json with one of the generators. With the JAVA/Kotlin API I do not see a way to support legacy API version(s) - and to be honest, I do also not like the complex .pom and runtime dependencies required to make the thing working with pure jvm. @GiviMAD with kind regards, |
This pull request has been mentioned on openHAB Community. There might be relevant details there: |
Hey @pgfeller sorry for the late reply, yes the proposal seems good to me. It will be great to remove the SDK because the fact that it uses kotlin makes the add-on very large, also keeping support for old versions will be great. |
Hi - thanks for your reply and the offer help 🙂. Don't worry about response times - first of all, it's a hobby and you invest your free time, and I also have limited resources I can spend on those projects. Therefore things take ages if I do them ... - so, that will be the same here; the hue sync binding took almost 12 months - but I hope that one will not take that long. We'll see - I rather invest a bit more to learn new things, experimenting - and hopefully produce a result that will be maintainable and scalable. As I tend to solve problems that affect me in the limited time - so it is important, that others could change and understand the code as well. That said: First I'll have to learn how to generate an API layer using the json/yaml definitions provided by Jellyfin. That takes time, but I do not think - there's much you could help me with that. Question here is if we go with "swagger" or "openAPI" ... "swagger" seems to be backed by a company; which has pro and cons. I did not evaluate them on a technical level. But do you prefer one over the other? Then I'll focus on that one ... If it is possible to generate the API layer I assume we'll miss some logic, that the SDK currently offers (e.g. server discovery). But if we generate code the first question is for me: How do we document the process and where do we store the utility scripts used for this. The current structure does not provide an answer for this as far as I know. But one thing at a time ... until we know that the API generated code, I think dividing work is premature. But in case that works I see e.g. the following things:
But the main tasks we have both at the moment is to enjoy christmas time and welcome the new year together with our loved ones! So enjoy an all the best! I'll keep you posted (btw. I like cats as well - two of my family members are feline as well). |
Signed-off-by: Patrik Gfeller <[email protected]>
Signed-off-by: Patrik Gfeller <[email protected]>
Fixes #17674
....
OpenAPI
Minimum Versions for API
Tasks
➡️ a 1st try to generate the JAVA API for 10.8 did not work out of the box (well, what did you expect 😉. During the investigation I found a fork of the library; which seems to be more community driven: OpenAPI Generator according to their Q&A. I decided to give this alternative a look before I invest too much time into swagger (task on hold)
References