feat(coverart): coverart builder to make specific queries to coverart api

[ritiek]

Ideally this will allow us to make specific calls to the coverart api. Such as to fetch front coverart for some release in 500px:

// https://coverartarchive.org/release/76df3287-6cda-33eb-8e9a-044b5e15ffdd/front-500
let in_utero_coverart = Release::fetch_coverart()
    .id("76df3287-6cda-33eb-8e9a-044b5e15ffdd")
    .front()
    .res_500()
    .execute()
    .expect("Unable to get cover art");

I'm not sure what the return type in our library should be here. The coverart api simply returns an image for this query.

Should we also return an image (maybe through the image crate), or maybe just return the redirect url from the original query (which looks like https://ia802607.us.archive.org/32/items/mbid-76df3287-6cda-33eb-8e9a-044b5e15ffdd/mbid-76df3287-6cda-33eb-8e9a-044b5e15ffdd-829521842_thumb500.jpg)?

Also, currently FetchCoverartQuery::execute() expects response content to be in json, but making these specific coverart calls does not return json. Not sure what the best way to deal with this in our library is.

[oknozor]

Hello @ritiek I won't have time to review this before next week.

Also I had a message from Yvan telling me that we need to give sign of life if we want to continue in the gsoc program.

I will contact you next week.

[ritiek]

Hello @ritiek I won't have time to review this before next week.

I see. Hope you're doing fine. Is there anything else you'd specifically want to me to work on until then? Otherwise I plan on implementing auto-retries for now as this feels something I wouldn't require much consulting on.

Also I had a message from Yvan telling me that we need to give sign of life if we want to continue in the gsoc program.

Totally understandable where this is coming from.

[oknozor]

Everything is fine, I am just super busy these days. Go for auto-retries !

[ritiek]

Good to know! Alright.

[oknozor]

This will work but it will allow to build invalid requests. For instance I could write :

Release::fetch_coverart()
    .id("1234")
    .back()
    .front()
    .res_1200()
    .res_500(); 

I see several ways to solve this :

• Wrapping the request in an intermediate builder returned by the front and back. This builder would implement the res_ functions.
• An alternative is to push the parameters (front, back and res) as some enum variant and check just before sending the request if all params are compatible.

I think I prefer the second approach because it allows to produce a detailed error message while being simple to implement and easy to extend.

[ritiek]

An alternative is to push the parameters (front, back and res) as some enum variant and check just before sending the request if all params are compatible.

Sounds good. How should I validate the request before sending it? Should I create another method (validate()?) that gets called as a part of execute() and checks whether the parameters are ordered correctly and are without repetition?

[oknozor]

Calling a validate() function as part of execute would do it, I don't think you need to check how method call are ordered since we can reorder everything when building the request. You only need to check for repetition.

[ritiek]

Alright. I've pushed new changes. What do you think?

[oknozor]

Also we need to update the readme.

[ritiek]

Also we need to update the readme.

Ok. Should I mention about the coverart feature in the features section in readme? And is it okay if I do this in a later PR? I'll also cover about get_coverart method and anything else worth mentioning about (retries?) in a later dedicated PR, if it's alright to you.

[oknozor]

Also we need to update the readme.

Ok. Should I mention about the coverart feature in the features section in readme? And is it okay if I do this in a later PR? I'll also cover about get_coverart method and anything else worth mentioning about (retries?) in a later dedicated PR, if it's alright to you.

Sure it's fine

[oknozor]

I don't think we need to panic here, we could just emit a warning to the standard output saying the call to res_xxx has been ignored

[ritiek]

I pushed a new commit to print these messages to stdout instead of panicking.

[oknozor]

Same here we shall just emit a warning, and ignore the additional image type, don't panic :)

[ritiek]

Is there anything else that needs to be addressed? Otherwise, this should be good to merge!