andreztz / pyradios

Client for the Radio Browser API
https://api.radio-browser.info/
MIT License
62 stars 22 forks source link

there is no http header named "application/xml" #34

Closed marc-portier closed 2 years ago

marc-portier commented 2 years ago

a bit puzzled by this line:

https://github.com/andreztz/pyradios/blob/7d9416f2cb8f91f97de79751f45b6eff61ab5f22/pyradios/radios.py#L27

it looks like the test on this line will always yield False, effectively calling resp.json() even when the fmt="xml" which, considering that the response-output should be of type XML, shall brake the json-decoding --> thus effectively making it impossible to use the RadioBrowser(fmt="xml")

--> using the above in e.g. the tests on this line https://github.com/andreztz/pyradios/blob/master/tests/test_radio_browser.py#L12 actually shows these failing json decodings all over

I guess the test should be checking rather one of:

the latter seems to be the safer option, although asserting that it conforms to the self.__content_type might be wise as well.

However:

One step further down the rabbit hole though: returning resp.content does not make sense does it? I mean it totally does not match the service interface of having a RadioBrowser(fmt="json") -- I find it very counter-intuitive to have a simply config-setting in the constructor of an object yielding the effect that the user of the service no longer gets decent python object returns, but rather a string holding the xml-content that the client now has to parse and handle itself ??

In other words, even after fixing the line as suggested, one would still need a completely separate tests/test_radio_browser_in_xml_modus.py to deal with that different set of responses.

Since the fmt="xml" does not add value to the python user, why would she ever use it? And if it is not to be used, why have it available? I understand how the service providers of radio-browser.info want to cater for different flavors of end-users - but users of a python library like this just want to get a hold of simple python native object-structures, and are not interested in how things got serialized over the interwebs?

We should discuss, but the best fix for this IMHO is to ignore support for the fmt="xml" all together, and just remove all traces of it from the code?

andreztz commented 2 years ago

Yes, the best solution is to remove support for XML. This task was already on my To-do list. I'll do that in the next few days, feel free to point out improvements. Thanks @marc-portier