-
Notifications
You must be signed in to change notification settings - Fork 3
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
hardcoded version string (v3 doesn't have json) #14
Comments
Good picklup - wow this is ancient and a bit of cut and pasting to blame here. I will look at moving this duplicated code into one shared function and switch to v5. It was probably written before 5 was released (or copied an example from docs). All the calls are in json so no sense even hard coding 3. |
Have now removed these hardcoded versions in the API calls. Unnecessary and straight up copied from the API docs here: https://ampache.org/api/api-5/#sending-handshake-request |
awesome, i'll update the examples which are ALSO just copy pastes! (website updated now too) |
So is the demo at http://foamdemo.mcgee.technology/ still using the unfixed version? Because I can see, that it makes handshake with the version 350001 but it works only if the server responses follow the APIv5 conventions. I'm developer of another Ampache-compatible server, and in my current development version, I was returning APIv5-compatible responses only if the version requested by the client is 5+. |
that's the way it should be for 5.2+ when we supported one api version it didn't matter but after that release there is support for version based on handshake. ping can also change the version if needed. (no idea why you'd need to but it's possible.) |
Maybe I misunderstand your point @lachlan-00, but no, I don't think that this is the "way it should be". Your original point from the OP is valid and it's not good to ask for JSON API 3.x.x as that doesn't exist and then the client depends on unspecified operation. But http://foamdemo.mcgee.technology/ still does that, so presumably it's running the latest release v0.6.2-beta which doesn't contain the commit 8d93145. On the other hand, https://demo.ampache.dev/foam/ apparently runs a newer development version as it doesn't specify any API version on But I don't think that not specifying the API version in |
One thing i noticed. Calls using a v3 api string (350001) will force to v5.
do you code against v5 of the api? might be better to set this to a 5.5.3 or 553000 to make sure you get the right responses
I found a few with a hardcoded v3 version
includes/playlistAPI.php
includes/callAPI.php
includes/favAPI.php
includes/playlistfromqAPI.php
The text was updated successfully, but these errors were encountered: