crustymonkey / py-sonic

A python library to wrap the Subsonic REST API
http://stuffivelearned.org/doku.php?id=programming:python:py-sonic
GNU General Public License v3.0
56 stars 26 forks source link

fix: remove '.view' from the api endpoints #30

Closed TravonteD closed 1 year ago

TravonteD commented 1 year ago

The subsonic spec for version 1.16.1 does not include '.view' including in the url breaks for implementations that only expect exact matches from the spec.

crustymonkey commented 1 year ago

Thanks for the pull request here. I did a little additional cleanup on top of your changes (removed the methodName entirely). Appreciate the work :-)

alxrdn commented 1 year ago

Hi, I'm using py-sonic against Ampache implementation of subsonic API. Ampache seems to support only subsonic spec version up to v1.13.0. Since this change (removing ".view" from the url), Ampache is not able to match with correct path anymore (returns 404).

I'm not sure if this ".view" extension is part of the 1.13.0 spec (?). If so, it would be great if py-sonic could handle the retrocompatibility, based on version parameter value. Otherwise, I'll be happy to raise the point on Ampache dev side.

Thanks

crustymonkey commented 1 year ago

@alxrdn Thanks for flagging this. For now, I would suggest using version 0.7.9 from pypi or the 0.7.9 tag here. I'll roll back the changes to remove the .view as having it there wasn't something that was actually a problem with Subsonic.

alxrdn commented 1 year ago

@crustymonkey No problem. I didn't mean to suggest to rollback your code: if the Subsonic specs don't require this suffix, then it should not be there. To be honest I don't even understand why Ampache built their API routes with it...

Meanwhile (and for anyone with the same problem), I mitigated the issue by rewriting the URL in my front webserver (nginx) in order to add the .view suffix if it's missing: rewrite ^(.*)(?<!\.view)$ $1.view;

crustymonkey commented 1 year ago

Well, it doesn't require it, but nor does it disallow it. Therefore, if there's a compatibility issue, it should be rolled back :-)

crustymonkey commented 1 year ago

@alxrdn I've reverted the change and pushed a new version (1.0.0) to pypi. I figured it was time to actually go for a 1.0.0 here as it's been in beta for long enough ;-)