Add Libre.fm support. about rustfm-scrobble HOT 24 OPEN

Hey @shymega, sorry for taking so long to get back to you. Would be happy to add Libre.fm support. Not something I'd work on until post-v1.0 (I simply don't use Libre.fm), but always happy to take PRs if you're still interested in adding this.

from rustfm-scrobble.

No problem about the delay.

I'm just looking at the code now. Looking in particular at the source file src/scrobbler.rs, function new on line 20.

The problem is where to specify the endpoint. I suppose we could add an endpoint field to the client struct, and allow the developer to specify the endpoint after creating the an instance of client, which would also mean extra line(s) of code as well.

The alternative is to specify another argument to new() for the endpoint, which, IMHO, would not be desirable - it would require existing usages to take that into account, I think...

Personally, I think putting an endpoint field to the client struct is the best approach. What are your thoughts?

from rustfm-scrobble.

I just tried my approach I outlined in my last comment, which compiles fine, but Libre.fm doesn't seem to recognise the api_key & api_secret parameters. Indeed, my current scrobbler - mpdscribble - only needs a username and password in the configuration file.

Its possible this isn't going to be a quick fix. I've tried to find some documentation for Libre.fm, but it is lacking.

from rustfm-scrobble.

Looks like they don't require an API key & secret - which I guess makes sense for a free & open platform? If that's the case, it may be a fiddle of a refactor.

from rustfm-scrobble.

Yeah, that's the conclusion I'm coming to here as well. I had hoped it would be as simple as a endpoint update, but it doesn't look that simple. As well as that, the website doesn't have an API registration page either.

It might be easier to develop a whole new library for Libre.fm, but that's extra dependencies for developers..

from rustfm-scrobble.

Right, after asking in Libre.fm's IRC channel, API key and secret doesn't matter - we can just use a 32 digit placeholder.

from rustfm-scrobble.

If its as simple as a 32 digit random/static number for the key and secret, we can move forward with that.

I've had a look at implementing it in the existing codebase, but it is a bit hacky, and I haven't worked on it for a while... had some data loss, so I'm not even sure if I still have it.

from rustfm-scrobble.

HI @shymega, I'm happy to look over any PRs and work a bit on this but don't have a lot of time to dedicate. Maybe we could land this for Hacktoberfest?

from rustfm-scrobble.

@bobbo Sounds good to me. I might forget, but if you think I've forgotten, feel free to give me a prod!

Thanks.

from rustfm-scrobble.

Still taking a look at this. Apologies for the delay.. will keep you updated 😅

from rustfm-scrobble.

No worries at all @shymega, there's no rush! If you've got any questions / anything you want to run past me just shout, happy to lend a hand.

from rustfm-scrobble.

Hey @dmfutcher - I don't know if you saw @InputUsername's comment about Libre.fm support on the referenced issue, and their scrobbler is using an unmaintained Listenbrainz library,

What would you say about adding both Libre.fm and Listenbrainz support to this library? Would it be in the library's scope? Personally, I can see a unified approach being better and less confusing for new developers.

from rustfm-scrobble.

@shymega I believe many scrobbling services like ListenBrainz and Maloja support access through a Last.fm-like API, so adding configurable API URLs could potentially be a catch-all solution for that?

One thing that I did run into with my listenbrainz crate was that ureq (which rustfm-scrobble currently uses) sometimes errors out when connecting to api.listenbrainz.org -- and this seems to be an issue with ureq itself, so replacing ureq might be necessary for ListenBrainz support.

from rustfm-scrobble.

Potentially, but I'd also rather support the ListenBrainz native API, in case other hosted/self-hosted ListenBrainz instances don't have the Last.fm-like API. IMHO, it would be best to support their standard API. Unless I'm missing something here?

Interesting that ureq has a timeout issue. Have you measured the connection time when connecting to the endpoint? How long is it, if at all?

I'm planning to use reqwest for my bank library...

from rustfm-scrobble.

@shymega fair enough - in that case I'd be happy to accept PRs etc on listenbrainz-rs if adding native support to rustfm-scrobble isn't feasible.

I'm not sure it's a timeout issue for ureq, I vaguely remember there being some (long-standing) issues with supported TLS certs that pop up for some websites but not others. It caused about half my scrobbles to ListenBrainz to not go through when I tested it.

reqwest is fine - I personally prefer not to use it because it pulls in a bunch of crates from the Tokio ecosystem and doubles dependency count/compile times.

from rustfm-scrobble.

@shymega fair enough - in that case I'd be happy to accept PRs etc on listenbrainz-rs if adding native support to rustfm-scrobble isn't feasible.

I think it would be, it just might need refactoring...

I'm not sure it's a timeout issue for ureq, I vaguely remember there being some (long-standing) issues with supported TLS certs that pop up for some websites but not others. It caused about half my scrobbles to ListenBrainz to not go through when I tested it.

Hm. LMK if you remember anything else!I

reqwest is fine - I personally prefer not to use it because it pulls in a bunch of crates from the Tokio ecosystem and doubles dependency count/compile times.

Yeah, I have used it on my bank library, but as you said, it does pull in a bunch of Tokio crates. I just need a blocking HTTP client.

from rustfm-scrobble.

@shymega sorry for not getting back to you sooner - I've run into all of the following issues using ureq with ListenBrainz's API before: algesten/ureq#317, algesten/ureq#318, algesten/ureq#325

So, if rustfm-scrobble is turned into a unified scrobbler, I would recommend moving away from ureq

from rustfm-scrobble.

Hi all,

On the HTTP client, until very recently we were using reqwest but to sort out the huge dependency tree that comes with it, we switched to ureq. Is there a similarly minimal HTTP client that could be used in ureq's place? Or I would have no issue with a compile option that would swap reqwest back in (but wouldn't want that to be the default). If it helps, the last commit before ureq was switched in was 914c6f0364b.

Compile time options for enabling the other scrobbler backends may also be a good way of cutting out bloat if you only need Last.fm support vs supporting n-different scrobblers out of the box. Haven't really used them too much though, so could be totally unwieldy or unnecessary.

from rustfm-scrobble.

On the HTTP client, until very recently we were using reqwest but to sort out the huge dependency tree that comes with it, we switched to ureq. Is there a similarly minimal HTTP client that could be used in ureq's place? Or I would have no issue with a compile option that would swap reqwest back in (but wouldn't want that to be the default). If it helps, the last commit before ureq was switched in was 914c6f0364b.

I found isahc which wraps Curl but otherwise looks pretty similar to ureq. It doesn't use rustls so it avoids the issues ureq has.

Compile time options for enabling the other scrobbler backends may also be a good way of cutting out bloat if you only need Last.fm support vs supporting n-different scrobblers out of the box. Haven't really used them too much though, so could be totally unwieldy or unnecessary.

That sounds reasonable. The API of ListenBrainz is a bit different in that authenticated requests simply need a user token instead of requiring a more complex login flow, so I'm not sure how compatible it is with the way things are currently done in rustfm-scrobble.

AFAIK Libre.fm uses Last.fm's API though, so that would be a matter of allowing configurable API URLs, right?

from rustfm-scrobble.

from rustfm-scrobble.

Yeah, I'm wondering if it should be split out into a separate crate. My ideal scenario is one scrobbling crate fits all, but I think it might become hacky without a little refactoring.

Agreed, honestly I think it's best to keep rustfm-scrobble focused on Last.fm/Libre.fm and maintain a separate crate for ListenBrainz. I'd be happy to add you to listenbrainz-rs if you want.

from rustfm-scrobble.

I found isahc which wraps Curl but otherwise looks pretty similar to ureq. It doesn't use rustls so it avoids the issues ureq has.

isahc is async and relatively heavyweight compared to ureq, and requires C dependencies in the form of curl. If you're looking for something like ureq that allows using libraries other than rustls, I suggest attohttpc.

from rustfm-scrobble.

Looking at attohttpc, it looks just what I need for my scrobbler, and for the listenbrainz-rs crate as well. Thanks for the link 🙌🏻

@InputUsername Yep, let's do that then. We can always put a link to each other - vice versa - in the README, perhaps? Sorry for the late reply! If you could add me, that'd be great. We can then work on this enhancement there.

from rustfm-scrobble.

One thing that I did run into with my listenbrainz crate was that ureq (which rustfm-scrobble currently uses) sometimes errors out when connecting to api.listenbrainz.org

@InputUsername could you report that issue to ureq? api.listenbrainz.org supports modern cipher suites, so there should be nothing preventing ureq and/or rustls from working with it.

from rustfm-scrobble.